From 6b9cb13b07fa7c85243486d7cb8482ddb0776aaf Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 3 May 2026 18:40:18 -0700 Subject: [PATCH] fix: restore stats daemon deferral --- ...daemon-deferral-when-launching-playback.md | 67 +++++++++++++++++++ changes/327-stats-daemon-deferral.md | 4 ++ src/main/runtime/stats-server-routing.test.ts | 6 +- src/main/runtime/stats-server-routing.ts | 4 +- 4 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 backlog/tasks/task-327 - Restore-stats-daemon-deferral-when-launching-playback.md create mode 100644 changes/327-stats-daemon-deferral.md diff --git a/backlog/tasks/task-327 - Restore-stats-daemon-deferral-when-launching-playback.md b/backlog/tasks/task-327 - Restore-stats-daemon-deferral-when-launching-playback.md new file mode 100644 index 00000000..9753af94 --- /dev/null +++ b/backlog/tasks/task-327 - Restore-stats-daemon-deferral-when-launching-playback.md @@ -0,0 +1,67 @@ +--- +id: TASK-327 +title: Restore stats daemon deferral when launching playback +status: Done +assignee: + - '@Codex' +created_date: '2026-05-04 01:15' +updated_date: '2026-05-04 01:17' +labels: + - bug + - stats + - runtime +dependencies: [] +priority: high +--- + +## Description + + +Launching a video while a background stats daemon is already running must not fail with stats.serverPort already in use. Normal in-app stats startup should reuse the live daemon URL instead of binding a second stats server, while preserving managed playback shutdown behavior from TASK-316. + + +## Acceptance Criteria + +- [x] #1 A live background stats daemon from another process causes in-app stats URL resolution to return the daemon URL without starting a local stats server. +- [x] #2 Dead or stale daemon state is removed and local stats startup still works. +- [x] #3 Managed playback shutdown behavior remains covered by existing tests. +- [x] #4 Focused regression tests pass. + + +## Implementation Plan + + +1. Update `src/main/runtime/stats-server-routing.test.ts` first so a live foreign daemon must return its daemon URL and skip local server startup. +2. Run the focused routing test to confirm the regression fails red. +3. Update `src/main/runtime/stats-server-routing.ts` to return `{ source: 'background' }` for live foreign daemon state, clear stale/self-owned state, and keep local startup fallback unchanged. +4. Run focused stats routing tests plus managed playback tests touched by TASK-316. +5. Update changelog and task acceptance/final notes. + + +## Implementation Notes + + +Implemented via TDD: first changed `stats-server-routing.test.ts` to require live foreign daemon deferral and observed the expected red failure. Then restored `stats-server-routing.ts` to return the daemon URL with `source: 'background'` when daemon state belongs to a live other process. Stale/dead and self-owned stale cleanup paths remain local fallback. + +Verification passed: `bun test src/main/runtime/stats-server-routing.test.ts`; focused runtime suite for stats daemon + TASK-316 managed playback files; `bun run typecheck`; `bun run test:fast`. + +`bun run changelog:lint` is blocked by pre-existing unrelated `changes/326-anilist-time-position-post-watch.md` missing valid `type` metadata; `changes/327-stats-daemon-deferral.md` follows the expected fragment format. + + +## Final Summary + + +Summary: +- Restored in-app stats startup deferral to a live background stats daemon from another process, returning the daemon URL and skipping local stats server binding. +- Kept stale/dead daemon cleanup and local stats startup fallback behavior intact. +- Added a changelog fragment for the restored port-conflict fix. + +Verification: +- `bun test src/main/runtime/stats-server-routing.test.ts` +- `bun test src/main/runtime/stats-server-routing.test.ts src/core/services/mpv.test.ts src/core/services/mpv-protocol.test.ts src/main/runtime/mpv-client-event-bindings.test.ts src/main/runtime/mpv-main-event-bindings.test.ts src/main/runtime/mpv-main-event-main-deps.test.ts src/main/runtime/stats-cli-command.test.ts src/stats-daemon-control.test.ts` +- `bun run typecheck` +- `bun run test:fast` + +Blocked check: +- `bun run changelog:lint` fails on unrelated pre-existing `changes/326-anilist-time-position-post-watch.md` metadata, not this change. + diff --git a/changes/327-stats-daemon-deferral.md b/changes/327-stats-daemon-deferral.md new file mode 100644 index 00000000..9a6dd528 --- /dev/null +++ b/changes/327-stats-daemon-deferral.md @@ -0,0 +1,4 @@ +type: fixed +area: stats + +- Restored stats startup deferral to a running background stats daemon so video launches no longer fail when the stats port is already in use. diff --git a/src/main/runtime/stats-server-routing.test.ts b/src/main/runtime/stats-server-routing.test.ts index 75d2c754..496b3f30 100644 --- a/src/main/runtime/stats-server-routing.test.ts +++ b/src/main/runtime/stats-server-routing.test.ts @@ -36,14 +36,14 @@ function createHarness(options?: { }; } -test('stats server routing ignores a live background daemon from another process', () => { +test('stats server routing defers to a live background daemon from another process', () => { const { calls, handler } = createHarness({ state: { pid: 200, port: 7979, startedAtMs: 1 }, processAlive: true, }); - assert.deepEqual(handler(), { url: 'http://127.0.0.1:6969', source: 'local' }); - assert.deepEqual(calls, ['readBackgroundState', 'isProcessAlive', 'startLocalStatsServer']); + assert.deepEqual(handler(), { url: 'http://127.0.0.1:7979', source: 'background' }); + assert.deepEqual(calls, ['readBackgroundState', 'isProcessAlive']); }); test('stats server routing clears dead daemon state and starts local server', () => { diff --git a/src/main/runtime/stats-server-routing.ts b/src/main/runtime/stats-server-routing.ts index 0a92d2bb..b2a42149 100644 --- a/src/main/runtime/stats-server-routing.ts +++ b/src/main/runtime/stats-server-routing.ts @@ -14,7 +14,7 @@ function formatStatsServerUrl(port: number): string { return `http://127.0.0.1:${port}`; } -export type EnsureStatsServerUrlResult = { url: string; source: 'local' }; +export type EnsureStatsServerUrlResult = { url: string; source: 'background' | 'local' }; export function createEnsureStatsServerUrlHandler( deps: EnsureStatsServerUrlDeps, @@ -27,6 +27,8 @@ export function createEnsureStatsServerUrlHandler( deps.removeBackgroundState(); } else if (!deps.isProcessAlive(state.pid)) { deps.removeBackgroundState(); + } else if (state.pid !== deps.currentPid) { + return { url: formatStatsServerUrl(state.port), source: 'background' }; } if (!deps.hasLocalStatsServer()) {