mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 17:06:15 -08:00
fix(playback): clarify external player launch feedback (#1388)
This commit is contained in:
4gray authored and GitHub committed
1 parent
8442747c37
commit
d73acd6bfc
101 files changed
+8819
-850
No files matched your search
@@ -0,0 +1,492 @@
|
||||
# External Playback Launch Feedback Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Keep MPV/VLC recovery actions visible while reporting exact launch/session state in the diagnostic panel and global dock.
|
||||
|
||||
**Architecture:** Add a component-local, credential-free external recovery state machine beside `PlaybackRecoverySession`. The existing host output still owns source-specific launch payloads and binds its returned launch promise to the fieldless current intent; only that exact result supplies the initial session ID. `PORTAL_EXTERNAL_PLAYBACK` then supplies exact-ID status observation and close-before-switch. Attempts rerank but never remove external targets, while stale promises, sessions, timers, and diagnostic owners are rejected.
|
||||
|
||||
**Tech Stack:** Angular signals and control flow, Angular Material icons/spinner, Jest, Nx, Playwright Electron E2E, ngx-translate.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: External recovery state machine
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `libs/ui/playback/src/lib/web-player-view/external-playback-recovery.ts`
|
||||
- Create: `libs/ui/playback/src/lib/web-player-view/external-playback-recovery.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Write the failing state-machine tests**
|
||||
|
||||
Cover the exact public contract:
|
||||
|
||||
```typescript
|
||||
const state = new ExternalPlaybackRecovery();
|
||||
state.syncSession('content-a');
|
||||
const intent = state.begin('mpv');
|
||||
|
||||
expect(intent).not.toBeNull();
|
||||
expect(state.pending()).toBe(true);
|
||||
expect(state.target('mpv')).toMatchObject({
|
||||
status: 'launching',
|
||||
attempts: 1,
|
||||
sessionId: null,
|
||||
});
|
||||
expect(state.begin('vlc')).toBeNull();
|
||||
|
||||
expect(state.observe(session({ id: 'old-session', player: 'mpv' }))).toBe(
|
||||
false
|
||||
);
|
||||
expect(state.observe(session({ id: 'new-session', player: 'vlc' }))).toBe(
|
||||
false
|
||||
);
|
||||
expect(state.observe(session({ id: 'new-session', player: 'mpv' }))).toBe(
|
||||
false
|
||||
);
|
||||
expect(
|
||||
state.confirm(intent!, session({ id: 'new-session', player: 'mpv' }))
|
||||
).toBe(true);
|
||||
expect(state.target('mpv').status).toBe('started');
|
||||
```
|
||||
|
||||
Add separate tests for `playing`, `error`, `closed → idle`, stale exact-ID
|
||||
updates, timeout-to-error, stale timeout after session reset, retry attempt
|
||||
count, and `destroy()` timer cleanup.
|
||||
|
||||
- [ ] **Step 2: Run RED**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --runTestsByPath libs/ui/playback/src/lib/web-player-view/external-playback-recovery.spec.ts --skip-nx-cache
|
||||
```
|
||||
|
||||
Expected: FAIL because `external-playback-recovery.ts` does not exist.
|
||||
|
||||
- [ ] **Step 3: Implement the minimal signal state machine**
|
||||
|
||||
Use these exported shapes:
|
||||
|
||||
```typescript
|
||||
export type ExternalRecoveryStatus =
|
||||
'idle' | 'launching' | 'started' | 'playing' | 'error';
|
||||
|
||||
export interface ExternalRecoveryTargetState {
|
||||
readonly status: ExternalRecoveryStatus;
|
||||
readonly attempts: number;
|
||||
readonly sessionId: string | null;
|
||||
}
|
||||
|
||||
export interface ExternalRecoveryIntent {
|
||||
readonly token: symbol;
|
||||
readonly target: ExternalPlayerName;
|
||||
}
|
||||
|
||||
export class ExternalPlaybackRecovery {
|
||||
readonly states: Signal<
|
||||
Readonly<Record<ExternalPlayerName, ExternalRecoveryTargetState>>
|
||||
>;
|
||||
readonly pending: Signal<boolean>;
|
||||
syncSession(key: string): boolean;
|
||||
begin(target: ExternalPlayerName): ExternalRecoveryIntent | null;
|
||||
owns(intent: ExternalRecoveryIntent): boolean;
|
||||
confirm(
|
||||
intent: ExternalRecoveryIntent,
|
||||
session: ExternalPlayerSession
|
||||
): boolean;
|
||||
cancel(intent: ExternalRecoveryIntent): boolean;
|
||||
close(target: ExternalPlayerName, sessionId: string): boolean;
|
||||
observe(session: ExternalPlayerSession | null): boolean;
|
||||
fail(intent: ExternalRecoveryIntent): boolean;
|
||||
target(target: ExternalPlayerName): ExternalRecoveryTargetState;
|
||||
destroy(): void;
|
||||
}
|
||||
```
|
||||
|
||||
The internal launch timer is 10 seconds, and all ownership tokens are fieldless
|
||||
`Symbol()` values. Store only target, attempt count, status, and the exact
|
||||
session ID returned through the owned host launch promise.
|
||||
|
||||
- [ ] **Step 4: Run GREEN and refactor**
|
||||
|
||||
Run the Task 1 command again. Expected: all state-machine tests PASS.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add libs/ui/playback/src/lib/web-player-view/external-playback-recovery.ts libs/ui/playback/src/lib/web-player-view/external-playback-recovery.spec.ts
|
||||
git commit -m "feat(playback): track external recovery launches"
|
||||
```
|
||||
|
||||
### Task 2: Preserve and rerank external recommendations
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-recovery-policy.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.recovery.spec.ts`
|
||||
- Test: `libs/ui/playback/src/lib/web-player-view/web-player-recovery-policy.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Write failing ranking tests**
|
||||
|
||||
Add a focused spec for a helper with this contract:
|
||||
|
||||
```typescript
|
||||
const result = createWebPlayerRecommendations({
|
||||
...baseOptions,
|
||||
attemptedTargets: new Set(['videojs', 'mpv']),
|
||||
externalStates: {
|
||||
mpv: { status: 'error', attempts: 1, sessionId: 'mpv-1' },
|
||||
vlc: { status: 'idle', attempts: 0, sessionId: null },
|
||||
},
|
||||
});
|
||||
|
||||
expect(
|
||||
result.map((item) => (item.action === 'player' ? item.target : item.action))
|
||||
).toEqual(['vlc', 'mpv', 'alternative-source']);
|
||||
expect(result.map((item) => item.priority)).toEqual([
|
||||
'primary',
|
||||
'secondary',
|
||||
'secondary',
|
||||
]);
|
||||
```
|
||||
|
||||
Also prove that an attempted inline family remains excluded, ties preserve MPV
|
||||
before VLC, input arrays/sets/states are not mutated, and output stays capped at
|
||||
three.
|
||||
|
||||
- [ ] **Step 2: Run RED**
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --runTestsByPath libs/ui/playback/src/lib/web-player-view/web-player-recovery-policy.spec.ts --skip-nx-cache
|
||||
```
|
||||
|
||||
Expected: FAIL because external state is not accepted and attempted MPV is
|
||||
removed.
|
||||
|
||||
- [ ] **Step 3: Implement filtered policy attempts and stable reranking**
|
||||
|
||||
Pass only inline attempted targets into `recommendPlaybackRecovery()`. Stable
|
||||
sort adjacent external recommendations by `attempts`, using their original
|
||||
policy index as tie-breaker, then regenerate primary/secondary priority without
|
||||
mutating inputs.
|
||||
|
||||
- [ ] **Step 4: Update old integration expectations**
|
||||
|
||||
Change old assertions that expected MPV/VLC to disappear. They must now assert
|
||||
that both targets remain mounted and the less-attempted sibling is promoted.
|
||||
|
||||
- [ ] **Step 5: Run GREEN and commit**
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --runTestsByPath libs/ui/playback/src/lib/web-player-view/web-player-recovery-policy.spec.ts libs/ui/playback/src/lib/web-player-view/web-player-view.component.recovery.spec.ts --skip-nx-cache
|
||||
git add libs/ui/playback/src/lib/web-player-view/web-player-recovery-policy.ts libs/ui/playback/src/lib/web-player-view/web-player-recovery-policy.spec.ts libs/ui/playback/src/lib/web-player-view/web-player-view.component.recovery.spec.ts
|
||||
git commit -m "fix(playback): keep external recovery actions available"
|
||||
```
|
||||
|
||||
### Task 3: Wire launch ownership and close-before-switch
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.html`
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/playback-recovery-session.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/playback-recovery-session.spec.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/web-player-view/web-player-view.component.recovery.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Add failing integration tests**
|
||||
|
||||
Provide a fake `PORTAL_EXTERNAL_PLAYBACK` with `activeSession: signal(null)`,
|
||||
`visibleSession`, and `closeSession`. Prove:
|
||||
|
||||
```typescript
|
||||
mpvButton.click();
|
||||
mpvButton.click();
|
||||
expect(fallbackRequests).toHaveLength(1);
|
||||
expect(component.externalRecoveryPending()).toBe(true);
|
||||
|
||||
activeSession.set(
|
||||
externalSession({ id: 'mpv-1', player: 'mpv', status: 'opened' })
|
||||
);
|
||||
fixture.detectChanges();
|
||||
expect(component.externalRecoveryState().mpv.status).toBe('started');
|
||||
expect(playerActionIds()).toContain('playback-fallback-mpv');
|
||||
```
|
||||
|
||||
Add independent tests for close-before-VLC, refusing an unclosable active
|
||||
session, stale session after `playbackSessionKey` change, no duplicate output,
|
||||
and no URL/header fields in serialized coordinator state.
|
||||
|
||||
- [ ] **Step 2: Run RED**
|
||||
|
||||
Run the recovery integration spec. Expected: new assertions FAIL because the
|
||||
component does not observe external sessions.
|
||||
|
||||
- [ ] **Step 3: Wire the coordinator**
|
||||
|
||||
Inject `PORTAL_EXTERNAL_PLAYBACK` optionally, observe its active signal in an
|
||||
`effect`, reset external state from `syncRecoverySession()`, and destroy it from
|
||||
the existing component cleanup. For external targets:
|
||||
|
||||
1. validate the recommendation and current diagnostic;
|
||||
2. begin one owned external intent;
|
||||
3. if a nonterminal active external session exists, require `canClose`, await
|
||||
`closeSession`, and confirm it is no longer live;
|
||||
4. recheck intent and diagnostic ownership;
|
||||
5. record the external attempt and emit the existing exact
|
||||
`PlaybackFallbackRequest` once.
|
||||
|
||||
- [ ] **Step 4: Run GREEN and commit**
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --runTestsByPath libs/ui/playback/src/lib/web-player-view/playback-recovery-session.spec.ts libs/ui/playback/src/lib/web-player-view/web-player-view.component.recovery.spec.ts --skip-nx-cache
|
||||
git add libs/ui/playback/src/lib/web-player-view
|
||||
git commit -m "feat(playback): synchronize external launch feedback"
|
||||
```
|
||||
|
||||
### Task 4: Render accessible per-target feedback
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostic-panel/playback-diagnostic-panel.component.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostic-panel/playback-diagnostic-panel.component.html`
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostic-panel/playback-diagnostic-panel.component.scss`
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostic-panel/playback-recommendation-view.util.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostic-panel/playback-recommendation-view.util.spec.ts`
|
||||
- Modify: `libs/ui/playback/src/lib/playback-diagnostic-panel/playback-diagnostic-panel.component.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Write failing view/component tests**
|
||||
|
||||
Prove state-aware keys and labels:
|
||||
|
||||
```typescript
|
||||
expect(getRecommendationLabelKey(mpvRecommendation, launchingState)).toBe(
|
||||
'PLAYBACK_DIAGNOSTICS.ACTION_OPENING_MPV'
|
||||
);
|
||||
expect(getRecommendationLabelKey(mpvRecommendation, errorState)).toBe(
|
||||
'PLAYBACK_DIAGNOSTICS.ACTION_RETRY_MPV'
|
||||
);
|
||||
```
|
||||
|
||||
The component test must assert that the same MPV `HTMLButtonElement` remains in
|
||||
the DOM through state changes, keeps focus, contains a 16px spinner during
|
||||
launch, exposes `aria-busy="true"` and `aria-disabled="true"`, and does not emit
|
||||
when aria-disabled. Assert a visible polite status for started/playing/error.
|
||||
|
||||
- [ ] **Step 2: Run RED**
|
||||
|
||||
Run both panel specs. Expected: FAIL because the panel has no external-state
|
||||
input or spinner.
|
||||
|
||||
- [ ] **Step 3: Implement the view**
|
||||
|
||||
Import the existing Angular Material progress spinner. Keep external buttons
|
||||
mounted and use `aria-disabled` plus handler guards instead of native
|
||||
`disabled` for external handshake feedback, preserving keyboard focus. Keep
|
||||
the existing native `disabled` behaviour for inline switch/retry pending.
|
||||
|
||||
Add only transform/opacity/color transitions under 180ms and a
|
||||
`prefers-reduced-motion` rule that stops spinner animation when appropriate.
|
||||
|
||||
- [ ] **Step 4: Run GREEN and commit**
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --runTestsByPath libs/ui/playback/src/lib/playback-diagnostic-panel/playback-recommendation-view.util.spec.ts libs/ui/playback/src/lib/playback-diagnostic-panel/playback-diagnostic-panel.component.spec.ts --skip-nx-cache
|
||||
git add libs/ui/playback/src/lib/playback-diagnostic-panel
|
||||
git commit -m "feat(playback): show external launch action states"
|
||||
```
|
||||
|
||||
### Task 5: Keep dock errors visible and use exact statuses
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `apps/web/src/app/services/external-playback.service.ts`
|
||||
- Modify: `apps/web/src/app/services/external-playback.service.spec.ts`
|
||||
- Modify: `libs/ui/components/src/lib/external-playback-dock/external-playback-dock.component.ts`
|
||||
- Modify: `libs/ui/components/src/lib/external-playback-dock/external-playback-dock.component.html`
|
||||
- Modify: `libs/ui/components/src/lib/external-playback-dock/external-playback-dock.component.scss`
|
||||
- Modify: `libs/ui/components/src/lib/external-playback-dock/external-playback-dock.component.spec.ts`
|
||||
- Modify: `libs/workspace/shell/feature/src/lib/workspace-shell/workspace-shell.component.spec.ts`
|
||||
|
||||
- [ ] **Step 1: Write failing service and dock tests**
|
||||
|
||||
Change the error-session expectation from hidden to visible, then prove
|
||||
`dismissActiveSession()` hides it. Add dock cases for:
|
||||
|
||||
```typescript
|
||||
expect(statusText('launching')).toContain('Opening player');
|
||||
expect(statusText('opened')).toContain('Player started');
|
||||
expect(statusText('playing')).toContain('Playing');
|
||||
expect(errorDock.getAttribute('aria-live')).toBe('polite');
|
||||
expect(dismissButton.textContent).toContain('Dismiss');
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run RED**
|
||||
|
||||
```bash
|
||||
pnpm nx test web --runTestsByPath apps/web/src/app/services/external-playback.service.spec.ts --skip-nx-cache
|
||||
pnpm nx test components --runTestsByPath libs/ui/components/src/lib/external-playback-dock/external-playback-dock.component.spec.ts --skip-nx-cache
|
||||
```
|
||||
|
||||
Expected: service error visibility and dock status assertions FAIL.
|
||||
|
||||
- [ ] **Step 3: Implement exact dock semantics**
|
||||
|
||||
Hide only `closed` in `visibleSession`. Translate launching/opened/playing and
|
||||
generic error states. Keep detailed existing `session.error` for the visible
|
||||
error copy. Show “Close player” only for a closable live session and “Dismiss”
|
||||
for error. Keep the existing output so workspace shell routing does not change.
|
||||
|
||||
- [ ] **Step 4: Run GREEN and commit**
|
||||
|
||||
Run both commands plus the workspace-shell focused spec, then commit:
|
||||
|
||||
```bash
|
||||
git add apps/web/src/app/services/external-playback.service.ts apps/web/src/app/services/external-playback.service.spec.ts libs/ui/components/src/lib/external-playback-dock libs/workspace/shell/feature/src/lib/workspace-shell/workspace-shell.component.spec.ts
|
||||
git commit -m "fix(playback): keep external launch errors visible"
|
||||
```
|
||||
|
||||
### Task 6: Translation, documentation, and release note
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `apps/web/src/assets/i18n/*.json`
|
||||
- Modify: `docs/architecture/embedded-inline-playback.md`
|
||||
- Modify: `AGENTS.md`
|
||||
- Modify: `CLAUDE.md`
|
||||
- Create: `.changes/playback-external-launch-feedback.md`
|
||||
|
||||
- [ ] **Step 1: Add user-facing translation keys**
|
||||
|
||||
Add the same complete key set to every locale, translated where practical and
|
||||
English fallback otherwise:
|
||||
|
||||
```json
|
||||
"ACTION_OPENING_MPV": "Opening MPV…",
|
||||
"ACTION_OPENING_VLC": "Opening VLC…",
|
||||
"ACTION_REOPEN_MPV": "Open MPV again",
|
||||
"ACTION_REOPEN_VLC": "Open VLC again",
|
||||
"ACTION_RETRY_MPV": "Try MPV again",
|
||||
"ACTION_RETRY_VLC": "Try VLC again",
|
||||
"EXTERNAL_OPENING": "Opening player…",
|
||||
"EXTERNAL_STARTED": "Player started",
|
||||
"EXTERNAL_PLAYING": "Playing",
|
||||
"EXTERNAL_FAILED": "External player error"
|
||||
```
|
||||
|
||||
Add workspace dock keys for opening, started, playing, failed, and dismiss.
|
||||
|
||||
- [ ] **Step 2: Update canonical docs**
|
||||
|
||||
Replace the v1 “external target disappears after attempt” contract with the
|
||||
new per-target state/reranking and exact external session correlation. Mirror
|
||||
the shared-player summary byte-for-meaning between `AGENTS.md` and `CLAUDE.md`.
|
||||
|
||||
- [ ] **Step 3: Add release note**
|
||||
|
||||
```markdown
|
||||
---
|
||||
type: fix
|
||||
area: playback
|
||||
---
|
||||
|
||||
MPV and VLC recovery buttons now stay available after an attempt and show launch progress, player-started, playback, and failure feedback. External-player errors remain in the bottom status bar until dismissed.
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Validate and commit**
|
||||
|
||||
```bash
|
||||
pnpm run i18n:validate
|
||||
pnpm run release:notes:validate
|
||||
git diff --check
|
||||
git add apps/web/src/assets/i18n docs/architecture/embedded-inline-playback.md AGENTS.md CLAUDE.md .changes/playback-external-launch-feedback.md
|
||||
git commit -m "docs(playback): document external launch feedback"
|
||||
```
|
||||
|
||||
### Task 7: Electron regression flow
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `apps/electron-backend-e2e/src/dash-clearkey.e2e.ts`
|
||||
|
||||
- [ ] **Step 1: Change the existing E2E expectation before production code is considered complete**
|
||||
|
||||
The fixture IPC handler must send repository-owned `launching`, `opened`, and
|
||||
failure session updates through `EXTERNAL_PLAYER_SESSION_UPDATE`, and implement
|
||||
the close handler without starting real players.
|
||||
|
||||
Assert:
|
||||
|
||||
- MPV remains visible during and after the held handshake;
|
||||
- its spinner/status is visible while launching;
|
||||
- VLC cannot launch during the handshake;
|
||||
- after MPV starts, MPV remains available and VLC is primary;
|
||||
- the dock reads “Player started”;
|
||||
- a failed VLC session remains in the dock with a Dismiss action;
|
||||
- dismiss hides the dock while both diagnostic action buttons remain.
|
||||
|
||||
- [ ] **Step 2: Run the exact Electron target**
|
||||
|
||||
```bash
|
||||
pnpm nx run electron-backend-e2e:e2e-ci--src/dash-clearkey.e2e.ts --skip-nx-cache
|
||||
```
|
||||
|
||||
Expected: PASS with the repository-owned fixture and stubbed IPC only.
|
||||
|
||||
- [ ] **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add apps/electron-backend-e2e/src/dash-clearkey.e2e.ts
|
||||
git commit -m "test(playback): cover external launch feedback"
|
||||
```
|
||||
|
||||
### Task 8: Full validation, review, and PR
|
||||
|
||||
**Files:**
|
||||
|
||||
- Review all changed files from `origin/master...HEAD`.
|
||||
|
||||
- [ ] **Step 1: Run affected unit, lint, build, and policy validation**
|
||||
|
||||
```bash
|
||||
pnpm nx test ui-playback --skip-nx-cache
|
||||
pnpm nx test components --skip-nx-cache
|
||||
pnpm nx test web --skip-nx-cache
|
||||
pnpm nx affected -t lint --base=origin/master --head=HEAD --skip-nx-cache
|
||||
pnpm nx build web --configuration=production --skip-nx-cache
|
||||
pnpm nx build electron-backend --configuration=production --skip-nx-cache
|
||||
pnpm run i18n:validate
|
||||
pnpm run release:notes:validate
|
||||
pnpm run coverage:policy:check
|
||||
git diff --check origin/master...HEAD
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the targeted Electron E2E again from the final tree**
|
||||
|
||||
```bash
|
||||
pnpm nx run electron-backend-e2e:e2e-ci--src/dash-clearkey.e2e.ts --skip-nx-cache
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Perform local Codex P1/P2 review**
|
||||
|
||||
Run the local Codex CLI against `origin/master`, inspect every finding, add a
|
||||
failing regression test before each valid fix, and rerun the affected and full
|
||||
validation commands. Do not create the PR while an actionable P1/P2 remains.
|
||||
|
||||
- [ ] **Step 4: Push and create a ready PR**
|
||||
|
||||
Use title:
|
||||
|
||||
```text
|
||||
fix(playback): clarify external player launch feedback
|
||||
```
|
||||
|
||||
The PR body must summarize persistent recovery actions, exact session status,
|
||||
accessibility/privacy ownership, dock dismissal, docs/release note, and every
|
||||
validation command. Do not merge.
|
||||
|
||||
- [ ] **Step 5: Run the review loop**
|
||||
|
||||
Wait for all latest-head CI, Greptile 5/5, and Codex review. Fix actionable
|
||||
feedback with regression coverage, push, retrigger both reviewers, and repeat
|
||||
until the ready PR is merge-ready.
|
||||
@@ -0,0 +1,172 @@
|
||||
# External Playback Launch Feedback Design
|
||||
|
||||
## Goal
|
||||
|
||||
Make MPV and VLC recovery actions reversible and understandable. A target stays
|
||||
available after an attempt, the diagnostic panel reports launch progress and
|
||||
outcome, and the workspace dock remains the authoritative global view of the
|
||||
external-player session.
|
||||
|
||||
## Current Problem
|
||||
|
||||
`WebPlayerViewComponent` records MPV or VLC as attempted before emitting the
|
||||
fallback request. The recommendation policy then removes attempted targets, so
|
||||
each button disappears even though IPTVnator only knows that the action was
|
||||
requested. It does not yet know whether the external player started or played
|
||||
the stream.
|
||||
|
||||
The external-player session already reports `launching`, `opened`, `playing`,
|
||||
`error`, and `closed`, but the dock hides `error`, renders `opened` and
|
||||
`playing` as the same “Opened” status, and exposes no dismiss action for an
|
||||
external-player error.
|
||||
|
||||
## Considered Approaches
|
||||
|
||||
### 1. Keep actions and show only a local timed spinner
|
||||
|
||||
This is the smallest change, but the UI would guess when launching finished
|
||||
and could disagree with the Electron session. It is rejected because the app
|
||||
already has structured launch state.
|
||||
|
||||
### 2. Use only callbacks on every `PlaybackFallbackRequest` host
|
||||
|
||||
Every M3U, Xtream, Stalker, and shared host could make the launch promise the
|
||||
only feedback source. This supplies an exact initial session ID, but by itself
|
||||
still misses later `playing` and `error` session updates.
|
||||
|
||||
### 3. Correlate the owned launch result with the shared external session
|
||||
|
||||
This is the selected approach. The web-player component keeps source launch
|
||||
ownership in its existing host output. Its credential-free tracker binds the
|
||||
host's returned launch promise to the current fieldless intent token, and only
|
||||
that exact result may supply the initial session ID. Later status changes come
|
||||
from `PORTAL_EXTERNAL_PLAYBACK.activeSession` and must match that ID. This
|
||||
prevents a delayed timed-out attempt from taking over a retry. No URL, headers,
|
||||
DRM data, credentials, or raw diagnostic payload enters the ownership state.
|
||||
|
||||
## State Model
|
||||
|
||||
Each external target has component-local state:
|
||||
|
||||
- `idle`: no launch is in flight; a previous successful/closed attempt may
|
||||
still be represented by an attempt count;
|
||||
- `launching`: the request is being handed off or Electron is spawning the
|
||||
player;
|
||||
- `started`: Electron confirmed the process or existing player accepted the
|
||||
stream;
|
||||
- `playing`: MPV/VLC position polling confirmed playback;
|
||||
- `error`: the launch/session failed.
|
||||
|
||||
The state also contains an attempt count and, only after correlation, the exact
|
||||
external session ID. It never contains the stream URL, headers, credentials,
|
||||
engine messages, or error details. A fieldless `Symbol` intent plus the current
|
||||
content-session generation rejects stale completions and timers.
|
||||
|
||||
Changing `playbackSessionKey` resets both targets. Destroying the component
|
||||
invalidates the intent and clears its bounded launch timer. A later session
|
||||
update is accepted only if it belongs to the correlated session ID.
|
||||
|
||||
## Recommendation Behaviour
|
||||
|
||||
External attempts no longer remove MPV or VLC from the ranked action list.
|
||||
Inline engine attempts retain the current family-exclusion rules.
|
||||
|
||||
Within an otherwise unchanged policy result:
|
||||
|
||||
- an untried external target ranks ahead of an attempted sibling;
|
||||
- fewer attempts rank ahead of more attempts;
|
||||
- policy order remains the tie-breaker;
|
||||
- the first surviving action remains primary and the total remains capped at
|
||||
three.
|
||||
|
||||
Labels describe the action, while adjacent status copy describes the outcome:
|
||||
|
||||
| State | Action label | Status |
|
||||
| ----------------------- | ------------------ | --------------------- |
|
||||
| `idle`, never attempted | Open in MPV/VLC | none |
|
||||
| `launching` | Opening MPV/VLC… | Opening player… |
|
||||
| `started` | Open MPV/VLC again | Player started |
|
||||
| `playing` | Open MPV/VLC again | Playing |
|
||||
| `error` | Try MPV/VLC again | External player error |
|
||||
|
||||
Buttons remain mounted with stable recommendation keys, preserving layout and
|
||||
focus. During a launch handshake, recovery actions expose `aria-busy` and
|
||||
`aria-disabled`; activation handlers enforce the same single-flight guard, so
|
||||
keyboard or programmatic activation cannot duplicate the request. A small
|
||||
spinner replaces the external-action icon. State changes are announced through
|
||||
concise `aria-live="polite"` status text.
|
||||
|
||||
Before a recovery action opens another external player, it closes the currently
|
||||
tracked live external session through `PORTAL_EXTERNAL_PLAYBACK`. If the
|
||||
session is still live and cannot be closed, the new attempt fails locally
|
||||
instead of starting a second process. The coordinator applies the exact
|
||||
confirmed close synchronously before emitting the replacement, so coalesced
|
||||
Angular effects cannot leave stale Playing feedback. If diagnostic ownership
|
||||
changes while closing, the unlaunched intent is cancelled without reporting a
|
||||
launch error. MPV and VLC are available again after the handshake.
|
||||
|
||||
If no matching session arrives within a bounded timeout, the target transitions
|
||||
to `error`. The timeout is feedback for a missing handoff, not evidence about
|
||||
stream playback.
|
||||
|
||||
## Dock Behaviour
|
||||
|
||||
`ExternalPlaybackService.visibleSession` keeps error sessions visible until
|
||||
the user dismisses them or another launch begins. The dock maps session states
|
||||
without inference:
|
||||
|
||||
- `launching` → “Opening player…” with a spinner;
|
||||
- `opened` → “Player started”;
|
||||
- `playing` → “Playing”;
|
||||
- `error` → the existing session error or a localized generic external-player
|
||||
failure;
|
||||
- `closed` remains hidden by the global service.
|
||||
|
||||
The dock uses an `aria-live="polite"` status region and `aria-busy` while
|
||||
launching. A live closable session shows “Close player”; an error shows
|
||||
“Dismiss”. Dismiss only hides the terminal notification and never stores a
|
||||
retry payload, so headers and credentials are not retained for a dock retry.
|
||||
|
||||
## Error And Privacy Boundaries
|
||||
|
||||
The diagnostic action shows only localized, app-owned launch-state copy. It
|
||||
does not render rejected promise messages or external-player stderr. The dock
|
||||
may continue to show the existing session error supplied by the Electron
|
||||
boundary; it does not add new logging or persistence.
|
||||
|
||||
Recovery recommendations remain user-selected. There is no automatic retry,
|
||||
automatic player switch, persistent history, telemetry, or learning.
|
||||
|
||||
## Testing
|
||||
|
||||
Unit and component coverage will prove:
|
||||
|
||||
- external attempts stay visible and rerank without weakening inline
|
||||
engine-family exclusion;
|
||||
- one handshake at a time, duplicate activation rejection, stable DOM keys,
|
||||
spinner/labels, and `aria-busy`/`aria-disabled` feedback;
|
||||
- exact session-ID correlation, stale update/timeout rejection, session-key
|
||||
reset, close-before-switch, and unclosable-session failure;
|
||||
- dock `launching`, `opened`, `playing`, `error`, close, and dismiss states;
|
||||
- `ExternalPlaybackService` keeps errors visible until dismissal.
|
||||
|
||||
Electron E2E will extend the deterministic recovery fixture to assert that the
|
||||
buttons remain after an MPV attempt, VLC is promoted, the lower dock shows
|
||||
launch feedback, and an error remains dismissible. Existing playback fixtures
|
||||
and player stubs must be used; no real playlist, account, or external process is
|
||||
required.
|
||||
|
||||
## Documentation And Release Note
|
||||
|
||||
Update the canonical playback recovery architecture in
|
||||
`docs/architecture/embedded-inline-playback.md`, the shared-player summary in
|
||||
`AGENTS.md` and `CLAUDE.md`, and add a user-facing `fix(playback)` note under
|
||||
`.changes/`.
|
||||
|
||||
## Out Of Scope
|
||||
|
||||
- proving playback for sessions that do not expose position polling;
|
||||
- retry from the dock;
|
||||
- retaining playback headers or credentials after the original request;
|
||||
- automatic failover between MPV and VLC;
|
||||
- redesigning the diagnostic overlay or dock.
|
||||
Reference in new issue
Block a user