diff --git a/apps/electron-backend/src/app/events/external-player-process.spec.ts b/apps/electron-backend/src/app/events/external-player-process.spec.ts new file mode 100644 index 000000000..2329e36bf --- /dev/null +++ b/apps/electron-backend/src/app/events/external-player-process.spec.ts @@ -0,0 +1,36 @@ +import type { ChildProcess } from 'child_process'; +import { EventEmitter } from 'events'; +import { waitForExternalPlayerProcessExit } from './external-player-process'; + +function createMockChildProcess(): ChildProcess { + return Object.assign(new EventEmitter(), { + exitCode: null, + killed: false, + kill: jest.fn(() => true), + signalCode: null, + stderr: null, + stdout: null, + unref: jest.fn(), + }) as unknown as ChildProcess; +} + +describe('external player process teardown', () => { + it('does not treat a process error as confirmed exit', async () => { + const child = createMockChildProcess(); + child.on('error', () => undefined); + let settled = false; + const exit = waitForExternalPlayerProcessExit(child).then(() => { + settled = true; + }); + + child.emit('error', new Error('kill failed')); + await Promise.resolve(); + + expect(settled).toBe(false); + + Object.defineProperty(child, 'exitCode', { value: 0 }); + child.emit('exit', 0); + + await expect(exit).resolves.toBeUndefined(); + }); +}); diff --git a/apps/electron-backend/src/app/events/external-player-process.ts b/apps/electron-backend/src/app/events/external-player-process.ts index 461bdc93a..8126eaee2 100644 --- a/apps/electron-backend/src/app/events/external-player-process.ts +++ b/apps/electron-backend/src/app/events/external-player-process.ts @@ -14,11 +14,9 @@ export function waitForExternalPlayerProcessExit( return new Promise((resolve) => { const complete = () => { child.off('exit', complete); - child.off('error', complete); resolve(); }; child.once('exit', complete); - child.once('error', complete); }); } diff --git a/docs/superpowers/plans/2026-08-08-external-playback-launch-feedback.md b/docs/superpowers/plans/2026-08-08-external-playback-launch-feedback.md index 3e6053242..2e0aeb03c 100644 --- a/docs/superpowers/plans/2026-08-08-external-playback-launch-feedback.md +++ b/docs/superpowers/plans/2026-08-08-external-playback-launch-feedback.md @@ -13,6 +13,7 @@ ### 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` @@ -34,8 +35,12 @@ expect(state.target('mpv')).toMatchObject({ }); expect(state.begin('vlc', 'old-session')).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: '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(true); expect(state.target('mpv').status).toBe('started'); ``` @@ -60,11 +65,7 @@ Use these exported shapes: ```typescript export type ExternalRecoveryStatus = - | 'idle' - | 'launching' - | 'started' - | 'playing' - | 'error'; + 'idle' | 'launching' | 'started' | 'playing' | 'error'; export interface ExternalRecoveryTargetState { readonly status: ExternalRecoveryStatus; @@ -78,10 +79,15 @@ export interface ExternalRecoveryIntent { } export class ExternalPlaybackRecovery { - readonly states: Signal>>; + readonly states: Signal< + Readonly> + >; readonly pending: Signal; syncSession(key: string): boolean; - begin(target: ExternalPlayerName, previousSessionId: string | null): ExternalRecoveryIntent | null; + begin( + target: ExternalPlayerName, + previousSessionId: string | null + ): ExternalRecoveryIntent | null; owns(intent: ExternalRecoveryIntent): boolean; observe(session: ExternalPlayerSession | null): boolean; fail(intent: ExternalRecoveryIntent): boolean; @@ -108,6 +114,7 @@ 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` @@ -126,8 +133,9 @@ const result = createWebPlayerRecommendations({ }, }); -expect(result.map((item) => item.action === 'player' ? item.target : item.action)) - .toEqual(['vlc', 'mpv', 'alternative-source']); +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', @@ -171,6 +179,7 @@ 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` @@ -188,7 +197,9 @@ mpvButton.click(); expect(fallbackRequests).toHaveLength(1); expect(component.externalRecoveryPending()).toBe(true); -activeSession.set(externalSession({ id: 'mpv-1', player: 'mpv', status: 'opened' })); +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'); @@ -228,6 +239,7 @@ 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` @@ -240,10 +252,12 @@ git commit -m "feat(playback): synchronize external launch feedback" 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'); +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 @@ -277,6 +291,7 @@ 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` @@ -326,6 +341,7 @@ 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` @@ -347,7 +363,7 @@ English fallback otherwise: "EXTERNAL_OPENING": "Opening player…", "EXTERNAL_STARTED": "Player started", "EXTERNAL_PLAYING": "Playing", -"EXTERNAL_FAILED": "Could not start player" +"EXTERNAL_FAILED": "External player error" ``` Add workspace dock keys for opening, started, playing, failed, and dismiss. @@ -382,6 +398,7 @@ 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** @@ -418,6 +435,7 @@ 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** diff --git a/docs/superpowers/specs/2026-08-08-external-playback-launch-feedback-design.md b/docs/superpowers/specs/2026-08-08-external-playback-launch-feedback-design.md index b4c89b1f8..8d8e5b9b9 100644 --- a/docs/superpowers/specs/2026-08-08-external-playback-launch-feedback-design.md +++ b/docs/superpowers/specs/2026-08-08-external-playback-launch-feedback-design.md @@ -17,8 +17,8 @@ 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 a -failed launch. +`playing` as the same “Opened” status, and exposes no dismiss action for an +external-player error. ## Considered Approaches @@ -81,13 +81,13 @@ Within an otherwise unchanged policy result: 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 | Could not start player | +| 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 @@ -116,7 +116,8 @@ without inference: - `launching` → “Opening player…” with a spinner; - `opened` → “Player started”; - `playing` → “Playing”; -- `error` → the existing launch error or a localized generic failure; +- `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