Conversation
* feat(media-server): implement runtime runtime management system - add UV bootstrapper with cross-platform install, caching, progress, validation - manage Python venv lifecycle, dependency installs, cleanup, and phased progress - introduce MediaServerService with lifecycle control, health checks, and port management - extend FFmpeg service to handle FFprobe with unified downloads and version updates - build settings UI sections for media server and plugins with real-time status - wire up 38 IPC channels covering UV, venv, FFmpeg/FFprobe, and server control - update build configs, add ffprobe download script, auto-start server, and bump backend ref * test: Update tests * test(main): mock electron app for ipc handlers * Update src/renderer/src/pages/settings/FFprobeSection.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Apply suggestion from @coderabbitai[bot] Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * refactor(types): extract common Platform and Arch types to shared package - Create packages/shared/types/system.ts with Platform and Arch type definitions - Remove duplicate type definitions from FFmpegDownloadService - Remove duplicate type definitions from UvBootstrapperService - Import shared types from @shared/types/system in both services This change improves code maintainability by centralizing common system type definitions. * fix(media-server): clear port and notify frontend on process exit Ensure media server port is properly cleared and frontend is notified when the process stops or crashes to prevent stale port usage. Changes: - Update notifyPortChanged signature to accept number | null - Clear this.port to null in both normal stop and crash exit handlers - Notify frontend with null port when server stops - Update SessionService listener to handle null port notifications - Add appropriate logging for port clearing events This prevents the frontend from continuing to use expired ports after the media server has stopped, requiring manual app restart. * fix(settings): prevent setTimeout memory leak in MediaServerSection Fix potential memory leak and unmounted component state update in MediaServerSection by properly managing setTimeout lifecycle: - Add successTimeoutRef to store timeout handle - Clear timeout in effect cleanup on component unmount - Clear timeout before creating new one in handleInstall - Clear timeout in error handler to prevent orphaned timers - Set ref to null after clearing to maintain clean state This prevents "Can't perform a React state update on an unmounted component" warnings and ensures proper cleanup of async operations. Co-Authored-By: Claude <noreply@anthropic.com> * fix(media-server): improve status display during installation and startup - Add 'starting' status immediately after installation completes - Move fetchServerInfo() to finally block to ensure status refresh - Prevent confusing 'stopped' status display during startup process * fix(python-venv): preserve final install progress state for callers Remove finally block that immediately clears installProgress, preventing callers from reading final "completed" or "error" state. Instead: - Clear installProgress at start of new installation - Preserve final state after completion/error for caller consumption - Remove unnecessary finally block cleanup This ensures MediaServerSection and other callers can reliably read the final installation state. Co-Authored-By: Claude <noreply@anthropic.com> * refactor(types): centralize media server types to shared package - Create shared types module for Media Server and Python Venv - Add media-server.ts with MediaServerInfo, MediaServerStatus, PythonVenvInfo, and InstallProgress types - Remove duplicate type definitions from MediaServerService, PythonVenvService, and MediaServerSection - Update imports to use @shared/types across main and renderer processes - Maintain backward compatibility by re-exporting types from service files - All type checks passing
… videos (#205) * feat(player): add media server recommendation prompt for incompatible videos When a video format is incompatible and Media Server is not installed, show a friendly prompt recommending installation. Features: - Automatic incompatibility detection with codec checking - Smart navigation to Media Server section in settings - Smooth scroll and highlight animation for better UX - Auto-trigger installation with one click - Dark mode compatible icon styling - i18n support (English and Chinese) Implementation: - Created MediaServerRecommendationPrompt component - Enhanced PlayerPage with Media Server check logic - Updated PluginsSettings with URL parameter handling - Added ref forwarding to MediaServerSection - Implemented pulse animation for visual feedback - Fixed styled-components v4+ keyframes interpolation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Update src/renderer/src/pages/settings/MediaServerSection.tsx Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * refactor(MediaServerRecommendationPrompt): replace hardcoded pixel values with design system spacing tokens - Replace hardcoded 48px with SPACING.XXL for IconWrapper dimensions - Replace hardcoded 32px with SPACING.XL for BenefitIcon dimensions - Replace hardcoded 2px with SPACING.XXS for BenefitIcon margin-top - Improve consistency with design system architecture - Enhance maintainability and cross-theme compatibility Changes: - IconWrapper: width/height 48px → ${SPACING.XXL}px - BenefitIcon: width/height 32px → ${SPACING.XL}px, margin-top 2px → ${SPACING.XXS}px This refactor eliminates magic numbers in styled components, ensuring better alignment with the project's design system tokens and simplifying future theme adjustments * refactor(MediaServerSection): fix ref handling with proper destructuring pattern - Replace incorrect ref prop with proper ref destructuring and renaming - Import React namespace for ref type annotations - Rename destructured ref to forwardedRef for clarity - Update useImperativeHandle to use forwardedRef instead of direct ref prop - Add displayName for better debugging experience Changes: - Import React namespace in addition to named hooks - Destructure ref as forwardedRef from props using rest pattern - Extract other props (onDependencyReady, triggerInstall, onInstallTriggered) from props object - Update useImperativeHandle first parameter to use forwardedRef - Add MediaServerSection.displayName = 'MediaServerSection' This refactoring addresses the incorrect pattern of accepting ref as a regular prop, which violates React's ref handling conventions. The new pattern properly destructures and renames the ref while maintaining full functionality. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Replace static "加载中..." text with an animated sliding progress bar that provides better visual feedback during video loading. Changes: - Add LoadingBarContainer and LoadingBarProgress styled components - Implement sliding animation using CSS keyframes (1.5s ease-in-out) - Update LoadingContainer to use flex column layout with gap - Use Ant Design CSS variables for theme compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
…#207) This update brings the following enhancements to the backend: ## New Features ### HLS Window Preloading with Configurable Strategy - **Automatic Background Preloading**: Automatically preload previous N windows in the background when accessing HLS files - **Configurable Preload Count**: Add `preload_previous_windows` configuration option (default: 1, min: 0) - **Non-blocking Execution**: Preload tasks run in background without blocking user requests - **Smart Cache Detection**: Skip already-cached windows to optimize resource usage - **Priority Management**: Low priority (3) for preload tasks to avoid impacting active user requests - **Performance Metrics**: Returns (queued_count, cached_count) metrics for monitoring ## Technical Changes ### API Layer (`src/app/api/`) - **deps.py**: Add PreloadStrategy dependency injection provider (`get_preload_strategy`) - **v1/jit.py**: Auto-trigger preload on window access with `asyncio.create_task` ### Configuration (`src/app/config/`) - **manager.py**: Add `preload_previous_windows` setting (default: 1, min: 0) - **schemas.py**: Add `preload_previous_windows` field to TranscodeConfig ### Service Layer (`src/app/services/`) - **preload_strategy.py**: Implement `preload_previous_windows` method with: - Cache key validation to avoid redundant transcoding - Window ID calculation for previous windows - Low-priority task queuing - Comprehensive error handling and logging ## Benefits - **Improved User Experience**: Proactively loads likely-needed segments for smoother playback - **Performance Optimization**: Maintains performance for active requests through priority management - **Resource Efficiency**: Smart cache detection prevents unnecessary transcoding - **Flexibility**: Configurable preload count allows tuning based on use case (0 to disable) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
Walkthrough本次变更新增并集成了 FFprobe 下载与管理、UV 引导器、Python 虚拟环境与媒体服务器运行服务;扩展 IPC 通道与 preload API;在设置页新增 FFprobe/媒体服务器管理区块与联动安装流程;播放器在转码前检查媒体服务器;更新打包/构建过滤;添加文档子模块并移除旧 FFmpeg 文档。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Renderer UI
participant Preload as Preload API
participant Main as Main Process (IPC)
participant UV as UvBootstrapperService
participant Venv as PythonVenvService
participant MS as MediaServerService
participant Bin as FFmpeg/FFprobe Service
User->>Preload: mediaServer.start(config?)
Preload->>Main: IpcChannel.MediaServer_Start
Main->>Venv: checkVenvInfo()
alt venv exists
Main->>UV: checkUvInstallation()
Main->>Bin: getFFmpeg/FFprobePath()
Main->>MS: start(config)
MS-->>Main: started/failed
else venv missing
Main-->>Preload: false (not started)
end
Main-->>Preload: result
Preload-->>User: success/failure
note over MS,User: 端口变化
MS-->>Main: PortChanged
Main-->>Preload: IpcChannel.MediaServer_PortChanged
Preload-->>User: 更新端口
sequenceDiagram
autonumber
actor Viewer as PlayerPage
participant Preload as Preload API
participant Main as Main Process
participant Venv as PythonVenvService
participant Sess as SessionService (renderer)
Viewer->>Preload: pythonVenv.checkInfo()
Preload->>Main: IpcChannel.PythonVenv_CheckInfo
Main->>Venv: checkVenvInfo()
Venv-->>Main: info(exists: bool)
Main-->>Preload: info
Preload-->>Viewer: info
alt venv exists
Viewer->>Sess: create playback session
Sess-->>Viewer: playlist URL (动态端口)
else
Viewer-->>Viewer: 展示安装建议弹窗
Viewer-->>Viewer: 使用原始源
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/main/ipc.ts (1)
508-515: 修复 FFmpeg 下载进度/取消的参数传递
FFmpegDownloadService的getDownloadProgress与cancelDownload现在都要求第一个参数传入资源标识(参见FFmpegDownloadService.test.ts中的调用)。当前 IPC handler 仍沿用旧签名,把platform当成首参传进去,会让渲染进程查询/取消进度时直接命中错误分支,导致进度查询永远失败。请同步更新成新签名。ipcMain.handle( IpcChannel.FfmpegDownload_GetProgress, async (_, platform?: string, arch?: string) => { - return ffmpegDownloadService.getDownloadProgress(platform as any, arch as any) + return ffmpegDownloadService.getDownloadProgress('ffmpeg', platform as any, arch as any) } ) ipcMain.handle(IpcChannel.FfmpegDownload_Cancel, async (_, platform?: string, arch?: string) => { - return ffmpegDownloadService.cancelDownload(platform as any, arch as any) + return ffmpegDownloadService.cancelDownload('ffmpeg', platform as any, arch as any) })src/main/services/FFmpegDownloadService.ts (4)
300-317: 隐私与合规风险:启动时调用 ipinfo.io 进行地区探测,且失败时默认为中国镜像
- 未经用户同意对外部服务发起请求(泄露 IP、UA 等)存在合规/隐私风险。
- 探测失败默认切换中国镜像,可能影响非中国用户的可用性和性能。
建议:
- 提供显式开关与用户同意(首次运行弹窗或设置项),默认无需外呼;
- 将失败时的默认策略改为“全球镜像”;
- 缓存用户选择,避免每次启动外呼。
可调整失败回退:
- logger.warn('无法检测用户地区,使用默认镜像源', { error }) - this.useChinaMirror = true // 检测失败时默认使用中国镜像源 + logger.warn('无法检测用户地区,回退到全球镜像源', { error }) + this.useChinaMirror = false // 失败回退到全球镜像Also applies to: 321-342
1-7: 缺少下载完整性校验(SHA256 等),存在供应链风险当前未对下载包进行校验(文件可能被篡改/中间人攻击)。请至少支持可选 SHA256 校验,并在失败时拒绝安装。
示例实现思路(简化):
// 追加:计算文件 sha256 private async verifySha256(filePath: string, expected?: string): Promise<boolean> { if (!expected) return true const crypto = await import('node:crypto') const hash = crypto.createHash('sha256') const stream = fs.createReadStream(filePath) return await new Promise((resolve, reject) => { stream.on('data', (d) => hash.update(d)) stream.on('end', () => resolve(hash.digest('hex').toLowerCase() === expected.toLowerCase())) stream.on('error', reject) }) }在 download 后、extract 前调用:
- await this.extractFile(downloadPath, tempDir) + const ok = await this.verifySha256(downloadPath, version.sha256) + if (!ok) { + throw new Error('校验失败:SHA256 不匹配') + } + await this.extractFile(downloadPath, tempDir)并在版本配置中按需填充 sha256。
Also applies to: 626-697
337-341: 地区探测失败默认返回 'CN' 会误判,建议返回 'ZZ'/空并走全球镜像当前默认 CN 会强行使用中国镜像,非预期。建议失败返回空值并让 detectRegionAndSetMirror 设 useChinaMirror=false。
- return 'CN' // 默认返回 CN + return '' // 失败返回空,外层回退到全球镜像
961-989: 依赖外部解压工具(unzip/tar),需处理缺失场景或在日志中明确提示在部分 Linux 发行版/最简环境中可能缺少 unzip/tar。建议:
- 捕获 ENOENT 并在日志中提示安装依赖;
- 或引入纯 JS 解压库作为后备。
Also applies to: 991-1005
src/preload/index.ts (2)
209-218: getProgress 返回类型过于宽泛,建议与主进程统一(DownloadProgress)为提升类型安全,建议将 ffmpeg/ffprobe 的 getProgress 返回值改为共享的 DownloadProgress 类型(可将类型上移到 @shared/types 并复用)。
示例:
- 在 packages/shared 下新增 DownloadProgress 定义;
- 此处声明为 Promise<DownloadProgress | null>。
Also applies to: 236-248
664-671: 遵循统一日志规范,避免直接使用 console按项目规范应统一使用 loggerService。若在 preload 无法直接复用,可通过 ipc 向主进程记录,或封装一个 preload 侧 logger 代理。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (34)
.gitignore(1 hunks).gitmodules(1 hunks)backend(1 hunks)docs(1 hunks)docs/FFmpeg-Integration.md(0 hunks)electron-builder.yml(1 hunks)electron.vite.config.ts(2 hunks)packages/shared/IpcChannel.ts(1 hunks)packages/shared/types/index.ts(1 hunks)packages/shared/types/media-server.ts(1 hunks)packages/shared/types/system.ts(1 hunks)scripts/download-ffmpeg.ts(1 hunks)scripts/download-ffprobe.ts(1 hunks)src/main/__tests__/ipc.database.test.ts(2 hunks)src/main/index.ts(2 hunks)src/main/ipc.ts(2 hunks)src/main/services/FFmpegDownloadService.ts(23 hunks)src/main/services/MediaServerService.ts(1 hunks)src/main/services/PythonVenvService.ts(1 hunks)src/main/services/UvBootstrapperService.ts(1 hunks)src/main/services/__tests__/FFmpegDownloadService.test.ts(6 hunks)src/main/services/__tests__/UvBootstrapperService.test.ts(1 hunks)src/preload/index.ts(2 hunks)src/renderer/src/components/MediaServerRecommendationPrompt.tsx(1 hunks)src/renderer/src/components/index.ts(1 hunks)src/renderer/src/i18n/locales/en-us.json(1 hunks)src/renderer/src/i18n/locales/zh-cn.json(3 hunks)src/renderer/src/pages/player/PlayerPage.tsx(6 hunks)src/renderer/src/pages/settings/FFmpegSection.tsx(1 hunks)src/renderer/src/pages/settings/FFprobeSection.tsx(1 hunks)src/renderer/src/pages/settings/MediaServerSection.tsx(1 hunks)src/renderer/src/pages/settings/PluginsSettings.tsx(1 hunks)src/renderer/src/pages/settings/SettingsPage.tsx(2 hunks)src/renderer/src/services/SessionService.ts(5 hunks)
💤 Files with no reviewable changes (1)
- docs/FFmpeg-Integration.md
🧰 Additional context used
📓 Path-based instructions (5)
src/renderer/src/**/*.{ts,tsx,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
优先使用 CSS 变量,避免硬编码样式值(颜色等)
Files:
src/renderer/src/pages/settings/SettingsPage.tsxsrc/renderer/src/components/index.tssrc/renderer/src/pages/settings/PluginsSettings.tsxsrc/renderer/src/pages/settings/MediaServerSection.tsxsrc/renderer/src/components/MediaServerRecommendationPrompt.tsxsrc/renderer/src/services/SessionService.tssrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/renderer/src/pages/player/PlayerPage.tsx
src/renderer/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
尺寸与时长等不要硬编码,优先使用 useTheme() 的 token 或集中样式变量(如 motionDurationMid、borderRadiusSM/MD)
Files:
src/renderer/src/pages/settings/SettingsPage.tsxsrc/renderer/src/components/index.tssrc/renderer/src/pages/settings/PluginsSettings.tsxsrc/renderer/src/pages/settings/MediaServerSection.tsxsrc/renderer/src/components/MediaServerRecommendationPrompt.tsxsrc/renderer/src/services/SessionService.tssrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/renderer/src/pages/player/PlayerPage.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 定制 antd 组件样式优先使用 styled-components 包装 styled(Component),避免全局 classNames
项目的图标统一使用 lucide-react,不使用 emoji 作为图标
组件/Hook 顶层必须通过 useStore(selector) 使用 Zustand,禁止在 useMemo/useEffect 内部调用 store Hook
避免使用返回对象的 Zustand 选择器(如 useStore(s => ({ a: s.a, b: s.b })));应使用单字段选择器或配合 shallow 比较器
遵循 React「副作用与状态更新」规范:渲染纯函数、Effect 三分法、幂等更新、稳定引用、严格清理、禁止写回自身依赖、Provider 值 memo、外部状态 selector 稳定等
统一使用 loggerService 记录日志而不是 console
logger 使用示例中第二个参数必须为对象字面量(如 logger.error('msg', { error }))
任何组件或页面都不要写入 media 元素的 currentTime,播放器控制由编排器统一负责
在 styled-components 中:主题相关属性使用 AntD CSS 变量(如 var(--ant-color-bg-elevated));
在 styled-components 中:设计系统常量(尺寸、动画、层级、字体、毛玻璃等)使用 JS 常量(如 SPACING、BORDER_RADIUS、Z_INDEX、FONT_SIZES 等)
Files:
src/renderer/src/pages/settings/SettingsPage.tsxsrc/main/ipc.tssrc/renderer/src/components/index.tssrc/renderer/src/pages/settings/PluginsSettings.tsxpackages/shared/types/media-server.tsscripts/download-ffmpeg.tselectron.vite.config.tssrc/main/services/__tests__/UvBootstrapperService.test.tssrc/main/services/__tests__/FFmpegDownloadService.test.tssrc/renderer/src/pages/settings/MediaServerSection.tsxpackages/shared/types/index.tssrc/renderer/src/components/MediaServerRecommendationPrompt.tsxsrc/renderer/src/services/SessionService.tspackages/shared/types/system.tssrc/main/services/UvBootstrapperService.tssrc/renderer/src/pages/settings/FFmpegSection.tsxsrc/preload/index.tspackages/shared/IpcChannel.tssrc/renderer/src/pages/settings/FFprobeSection.tsxsrc/main/services/PythonVenvService.tssrc/main/index.tssrc/main/services/MediaServerService.tssrc/renderer/src/pages/player/PlayerPage.tsxsrc/main/__tests__/ipc.database.test.tssrc/main/services/FFmpegDownloadService.tsscripts/download-ffprobe.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
使用 Vitest 作为测试框架
Files:
src/main/services/__tests__/UvBootstrapperService.test.tssrc/main/services/__tests__/FFmpegDownloadService.test.tssrc/main/__tests__/ipc.database.test.ts
**/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Player 页面:统一在组件顶层使用 Zustand selector,禁止在 useMemo/useEffect 内调用 store Hook;useSubtitleEngine 通过参数传入 subtitles 等防御处理
Files:
src/renderer/src/pages/player/PlayerPage.tsx
🧬 Code graph analysis (15)
src/main/ipc.ts (4)
src/main/services/FFmpegDownloadService.ts (1)
ffmpegDownloadService(1050-1050)src/main/services/UvBootstrapperService.ts (1)
uvBootstrapperService(1185-1185)src/main/services/PythonVenvService.ts (1)
pythonVenvService(499-499)src/main/services/MediaServerService.ts (1)
mediaServerService(613-613)
src/renderer/src/pages/settings/PluginsSettings.tsx (1)
src/renderer/src/contexts/theme.context.tsx (1)
useTheme(124-124)
electron.vite.config.ts (1)
scripts/upload-assets.js (1)
fs(4-4)
src/main/services/__tests__/UvBootstrapperService.test.ts (1)
src/main/services/UvBootstrapperService.ts (2)
UvBootstrapperService(135-1182)DownloadProgress(23-30)
src/renderer/src/pages/settings/MediaServerSection.tsx (4)
src/renderer/src/services/Logger.ts (3)
loggerService(817-817)info(436-438)error(422-424)src/renderer/src/contexts/theme.context.tsx (1)
useTheme(124-124)packages/shared/types/media-server.ts (3)
MediaServerInfo(18-25)PythonVenvInfo(30-37)InstallProgress(42-46)src/renderer/src/infrastructure/styles/theme.ts (6)
BORDER_RADIUS(61-72)ANIMATION_DURATION(91-100)EASING(103-114)SPACING(43-58)FONT_WEIGHTS(11-22)FONT_SIZES(25-40)
src/renderer/src/components/MediaServerRecommendationPrompt.tsx (2)
src/renderer/src/components/index.ts (1)
MediaServerRecommendationPrompt(2-2)src/renderer/src/infrastructure/styles/theme.ts (4)
SPACING(43-58)BORDER_RADIUS(61-72)FONT_SIZES(25-40)FONT_WEIGHTS(11-22)
src/renderer/src/services/SessionService.ts (1)
src/renderer/src/services/Logger.ts (2)
loggerService(817-817)error(422-424)
src/main/services/UvBootstrapperService.ts (3)
src/renderer/src/services/Logger.ts (3)
loggerService(817-817)error(422-424)info(436-438)packages/shared/types/system.ts (2)
Platform(2-2)Arch(3-3)src/main/services/FFmpegDownloadService.ts (1)
DownloadProgress(25-32)
src/renderer/src/pages/settings/FFmpegSection.tsx (4)
src/renderer/src/services/Logger.ts (2)
loggerService(817-817)error(422-424)src/renderer/src/contexts/theme.context.tsx (1)
useTheme(124-124)src/renderer/src/pages/settings/AboutSettings.tsx (1)
SettingRowTitle(412-424)src/renderer/src/infrastructure/styles/theme.ts (6)
SPACING(43-58)BORDER_RADIUS(61-72)ANIMATION_DURATION(91-100)EASING(103-114)FONT_WEIGHTS(11-22)FONT_SIZES(25-40)
src/renderer/src/pages/settings/FFprobeSection.tsx (4)
src/renderer/src/services/Logger.ts (1)
loggerService(817-817)src/renderer/src/contexts/theme.context.tsx (1)
useTheme(124-124)src/renderer/src/pages/settings/AboutSettings.tsx (1)
SettingRowTitle(412-424)src/renderer/src/infrastructure/styles/theme.ts (6)
SPACING(43-58)BORDER_RADIUS(61-72)ANIMATION_DURATION(91-100)EASING(103-114)FONT_WEIGHTS(11-22)FONT_SIZES(25-40)
src/main/services/PythonVenvService.ts (3)
src/renderer/src/services/Logger.ts (3)
loggerService(817-817)error(422-424)info(436-438)packages/shared/types/media-server.ts (2)
InstallProgress(42-46)PythonVenvInfo(30-37)src/main/services/UvBootstrapperService.ts (1)
uvBootstrapperService(1185-1185)
src/main/index.ts (3)
src/main/services/PythonVenvService.ts (1)
pythonVenvService(499-499)src/main/services/MediaServerService.ts (1)
mediaServerService(613-613)src/renderer/src/services/Logger.ts (1)
error(422-424)
src/main/services/MediaServerService.ts (6)
src/renderer/src/services/Logger.ts (2)
loggerService(817-817)error(422-424)packages/shared/types/media-server.ts (2)
MediaServerStatus(8-13)MediaServerInfo(18-25)src/main/services/FFmpegDownloadService.ts (1)
FFmpegDownloadService(277-1047)src/main/utils/index.ts (1)
getDataPath(11-17)src/main/services/UvBootstrapperService.ts (1)
uvBootstrapperService(1185-1185)src/main/services/PythonVenvService.ts (1)
pythonVenvService(499-499)
src/renderer/src/pages/player/PlayerPage.tsx (2)
src/renderer/src/services/SessionService.ts (1)
SessionService(127-537)src/renderer/src/components/MediaServerRecommendationPrompt.tsx (1)
MediaServerRecommendationPrompt(23-119)
src/main/services/FFmpegDownloadService.ts (2)
packages/shared/types/system.ts (2)
Platform(2-2)Arch(3-3)src/main/services/UvBootstrapperService.ts (1)
DownloadProgress(23-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (18)
src/main/__tests__/ipc.database.test.ts (5)
1-7: 导入结构合理。类型导入和值导入的分离符合 TypeScript 最佳实践。使用
import type导入ElectronApp是正确的,因为它仅用于类型标注。
8-77: Mock 结构全面且组织良好。集中化的 electron 模块 mock 提供了丰富的 API 表面,包括 app、BrowserWindow、ipcMain、dialog、session、shell、systemPreferences 和 webContents。这种结构提高了可维护性并支持更广泛的测试场景。
245-264: 测试设置结构良好。测试套件的设置遵循了最佳实践:
- 使用
beforeEach和afterEach进行适当的清理- IPC 处理程序捕获机制设计合理
- 通过
getMockedApp()获取 mock 实例保持了一致性
270-962: 测试用例覆盖全面。测试套件为 Files、VideoLibrary 和 SubtitleLibrary DAO 的 IPC 处理程序提供了全面的覆盖:
- 成功场景测试
- 错误处理测试
- 边界条件测试(空结果、不存在的记录等)
- 使用嵌套的
describe块进行良好的组织每个测试都正确验证了:
- DAO 方法是否使用正确的参数调用
- 返回值是否符合预期
- 错误是否正确传播
964-1093: 事务替代方案测试记录了设计决策。测试套件清楚地记录了为什么事务处理程序被注释掉(IPC 限制无法直接传递函数),并通过组合 DAO 操作的测试演示了替代方案。这为未来的维护者提供了有价值的上下文。
错误处理测试(第 1046-1092 行)正确地展示了组合操作中间失败的场景,并注释说明了需要在应用层面实现事务逻辑。
backend (1)
1-1: 请确认子模块提交内容已独立过审当前改动仅更新子模块指针,PR 本身无法查看 176ca823ac2e6365f0721e43725c633b2d5aa9da 的实际改动,请确认该提交已经经过适当的代码审查。
src/renderer/src/components/index.ts (1)
2-2: LGTM!新增的
MediaServerRecommendationPrompt组件导出符合项目的导出模式。src/renderer/src/i18n/locales/zh-cn.json (3)
172-194: LGTM!新增的媒体服务器提示文案清晰且具有说服力,有助于引导用户完成安装流程。
328-328: LGTM!按钮文案从"下载 FFmpeg"改为"安装"更加简洁,且与新增的 FFprobe 插件的"安装"按钮保持一致。
398-439: LGTM!新增的 FFprobe 插件配置项结构与 FFmpeg 保持一致,翻译准确完整。
packages/shared/types/index.ts (1)
1-7: LGTM!统一的类型导出文件使得类型引用更加便捷,符合模块化设计原则。
src/main/index.ts (2)
17-18: LGTM!新增的服务导入清晰明确,为后续的自动启动逻辑提供支持。
146-162: 确认 Media Server 自动启动非阻塞逻辑
start已声明为async,未调用spawnSync/execSync,不会阻塞事件循环- 异常仅在内部被
catch记录并返回false,不会中断应用启动- 如需完全后台化,可去掉对
start()的await并在方法内捕获错误src/renderer/src/pages/settings/SettingsPage.tsx (1)
12-12: LGTM!将
FFmpegSettings重构为PluginsSettings的改动合理,更好地体现了插件管理的统一性。导入和路由配置均已正确更新。Also applies to: 74-74
packages/shared/types/system.ts (1)
1-3: LGTM!平台和架构类型定义清晰简洁,为跨平台支持提供了类型安全保障。
electron-builder.yml (1)
21-26: out 目录尚未生成,需先执行渲染进程构建(如 npm run build:renderer)以生成 out 文件夹,然后重新运行以下脚本检查文件列表是否完整:echo "=== out 目录结构 ===" fd . out --type f --max-depth 2 | head -20scripts/download-ffmpeg.ts (1)
38-38: 确认 FFmpeg 8.0 下载链接有效
curl 返回 HTTP 200。src/main/services/MediaServerService.ts (1)
543-559: 确认主进程 Node/Electron 版本支持全局 fetch 与 AbortSignal.timeout
waitForServer/checkServerHealth 使用 fetch 和 AbortSignal.timeout(Node.js v18+ 内置),请检查 package.json 中 engines.node ≥18 或 dependencies.electron ≥18,以保证主进程环境具备支持,避免运行时 ReferenceError。
| show: vi.fn(), | ||
| hide: vi.fn() | ||
| } | ||
| } as unknown as ElectronApp |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
考虑改进类型转换模式。
第 34 行的双重类型转换 as unknown as ElectronApp 和第 77 行 getMockedApp() 中的再次转换表明可能存在类型兼容性问题。
考虑以下改进方案:
const electronModuleMock = vi.hoisted(() => {
- const appMock = {
+ const appMock: Partial<ElectronApp> = {
getVersion: vi.fn(() => '1.0.0'),
isPackaged: true,
// ... 其他属性
- } as unknown as ElectronApp
+ }
return {
// ...
- app: appMock,
+ app: appMock as ElectronApp,
// ...
}
})
-const getMockedApp = () => electronModuleMock.app as ElectronApp
+const getMockedApp = (): ElectronApp => electronModuleMock.app这样可以:
- 使用
Partial<ElectronApp>明确表示 mock 是部分实现 - 减少类型转换次数
- 在
getMockedApp()的返回类型上标注而不是在实现中转换
Also applies to: 77-77
| const originalPlatform = process.platform | ||
| const originalArch = process.arch | ||
|
|
||
| Object.defineProperty(process, 'platform', { value: 'win32' }) | ||
| Object.defineProperty(process, 'arch', { value: 'x64' }) | ||
|
|
||
| const result = service.getUvPath() | ||
| expect(result).toContain('latest-win32-x64') | ||
| expect(result).toContain('uv.exe') | ||
|
|
||
| Object.defineProperty(process, 'platform', { value: originalPlatform }) | ||
| Object.defineProperty(process, 'arch', { value: originalArch }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
避免直接修改 process.platform/arch,改用显式参数以消除易碎性
在多数 Node 版本中,process.platform/arch 是只读/不可配置,Object.defineProperty 可能抛出错误并导致测试不稳定。getUvPath 支持入参,直接传入平台/架构即可。
建议替换为直接传参调用:
- const originalPlatform = process.platform
- const originalArch = process.arch
-
- Object.defineProperty(process, 'platform', { value: 'win32' })
- Object.defineProperty(process, 'arch', { value: 'x64' })
-
- const result = service.getUvPath()
+ const result = service.getUvPath('win32', 'x64')
expect(result).toContain('latest-win32-x64')
expect(result).toContain('uv.exe')
-
- Object.defineProperty(process, 'platform', { value: originalPlatform })
- Object.defineProperty(process, 'arch', { value: originalArch })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const originalPlatform = process.platform | |
| const originalArch = process.arch | |
| Object.defineProperty(process, 'platform', { value: 'win32' }) | |
| Object.defineProperty(process, 'arch', { value: 'x64' }) | |
| const result = service.getUvPath() | |
| expect(result).toContain('latest-win32-x64') | |
| expect(result).toContain('uv.exe') | |
| Object.defineProperty(process, 'platform', { value: originalPlatform }) | |
| Object.defineProperty(process, 'arch', { value: originalArch }) | |
| }) | |
| const result = service.getUvPath('win32', 'x64') | |
| expect(result).toContain('latest-win32-x64') | |
| expect(result).toContain('uv.exe') | |
| }) |
🤖 Prompt for AI Agents
In src/main/services/__tests__/UvBootstrapperService.test.ts around lines 71 to
83, the test mutates process.platform/process.arch using Object.defineProperty
which is fragile or throws in many Node versions; instead call service.getUvPath
with explicit platform and arch arguments (e.g., 'win32', 'x64'), remove the
Object.defineProperty calls and the restore steps, and keep the same assertions
that the returned path contains 'latest-win32-x64' and 'uv.exe'.
| it('should return correct path for darwin-arm64', () => { | ||
| const originalPlatform = process.platform | ||
| const originalArch = process.arch | ||
|
|
||
| Object.defineProperty(process, 'platform', { value: 'darwin' }) | ||
| Object.defineProperty(process, 'arch', { value: 'arm64' }) | ||
|
|
||
| const result = service.getUvPath() | ||
| expect(result).toContain('latest-darwin-arm64') | ||
| expect(result).toContain('uv') | ||
|
|
||
| Object.defineProperty(process, 'platform', { value: originalPlatform }) | ||
| Object.defineProperty(process, 'arch', { value: originalArch }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
同上:用入参替代修改全局 platform/arch
darwin-arm64 场景也直接传参,避免重写全局属性。
- const originalPlatform = process.platform
- const originalArch = process.arch
-
- Object.defineProperty(process, 'platform', { value: 'darwin' })
- Object.defineProperty(process, 'arch', { value: 'arm64' })
-
- const result = service.getUvPath()
+ const result = service.getUvPath('darwin', 'arm64')
expect(result).toContain('latest-darwin-arm64')
expect(result).toContain('uv')
-
- Object.defineProperty(process, 'platform', { value: originalPlatform })
- Object.defineProperty(process, 'arch', { value: originalArch })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should return correct path for darwin-arm64', () => { | |
| const originalPlatform = process.platform | |
| const originalArch = process.arch | |
| Object.defineProperty(process, 'platform', { value: 'darwin' }) | |
| Object.defineProperty(process, 'arch', { value: 'arm64' }) | |
| const result = service.getUvPath() | |
| expect(result).toContain('latest-darwin-arm64') | |
| expect(result).toContain('uv') | |
| Object.defineProperty(process, 'platform', { value: originalPlatform }) | |
| Object.defineProperty(process, 'arch', { value: originalArch }) | |
| }) | |
| it('should return correct path for darwin-arm64', () => { | |
| const result = service.getUvPath('darwin', 'arm64') | |
| expect(result).toContain('latest-darwin-arm64') | |
| expect(result).toContain('uv') | |
| }) |
🤖 Prompt for AI Agents
In src/main/services/__tests__/UvBootstrapperService.test.ts around lines 85 to
98, the test mutates global process.platform/process.arch to simulate
darwin-arm64; instead pass platform/arch as arguments and avoid touching
globals. Update the service.getUvPath implementation to accept optional
(platform?: string, arch?: string) parameters defaulting to
process.platform/process.arch if not provided, then change this test to call
service.getUvPath('darwin', 'arm64') and assert the same expectations; do not
modify process global properties in the test.
| describe('checkVenvExists', () => { | ||
| it('should return true when venv exists on Windows', () => { | ||
| const originalPlatform = process.platform | ||
| Object.defineProperty(process, 'platform', { value: 'win32' }) | ||
|
|
||
| mockFs.existsSync.mockReturnValue(true) | ||
|
|
||
| const result = service.checkVenvExists('/test/project') | ||
| expect(result).toBe(true) | ||
| expect(mockFs.existsSync).toHaveBeenCalledWith( | ||
| path.join('/test/project', '.venv', 'Scripts/python.exe') | ||
| ) | ||
|
|
||
| Object.defineProperty(process, 'platform', { value: originalPlatform }) | ||
| }) | ||
|
|
||
| it('should return true when venv exists on Unix', () => { | ||
| const originalPlatform = process.platform | ||
| Object.defineProperty(process, 'platform', { value: 'darwin' }) | ||
|
|
||
| mockFs.existsSync.mockReturnValue(true) | ||
|
|
||
| const result = service.checkVenvExists('/test/project') | ||
| expect(result).toBe(true) | ||
| expect(mockFs.existsSync).toHaveBeenCalledWith( | ||
| path.join('/test/project', '.venv', 'bin/python') | ||
| ) | ||
|
|
||
| Object.defineProperty(process, 'platform', { value: originalPlatform }) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
测试 venv 场景请避免重写 process.platform;建议改造接口或使用更安全的桩
checkVenvExists 依赖 process.platform。直接重写全局属性风险大。建议:
- 方案A(推荐):为 checkVenvExists 增加可选 platform 入参,仅供测试使用;
- 方案B:在测试中抽离路径拼接逻辑进行断言,或以模块边界为单位改测上层调用。
如采用方案A,接口改为 checkVenvExists(projectPath: string, platform: NodeJS.Platform = process.platform)。随后测试可传入 'win32'/'darwin',无需污染全局。
| describe('Progress tracking', () => { | ||
| it('should report download progress with correct status transitions', async () => { | ||
| const statusSequence: string[] = [] | ||
| const progressCallback = vi.fn((progress: DownloadProgress) => { | ||
| statusSequence.push(progress.status) | ||
| }) | ||
| const mockRequest = { on: vi.fn(), destroy: vi.fn() } | ||
| const mockResponse = new PassThrough() | ||
| ;(mockResponse as any).statusCode = 200 | ||
| ;(mockResponse as any).headers = { 'content-length': '10240' } | ||
|
|
||
| httpsGetSpy.mockImplementation((_url, _options, callback) => { | ||
| setTimeout(() => { | ||
| callback(mockResponse) | ||
| setTimeout(() => { | ||
| mockResponse.write(Buffer.alloc(5120)) | ||
| setTimeout(() => { | ||
| mockResponse.write(Buffer.alloc(5120)) | ||
| mockResponse.end() | ||
| }, 1100) | ||
| }, 10) | ||
| }, 10) | ||
| return mockRequest | ||
| }) | ||
|
|
||
| const immediateCloseChild = { | ||
| on: vi.fn((event, callback) => { | ||
| if (event === 'close') { | ||
| callback(0) | ||
| } | ||
| }) | ||
| } | ||
| const slowCloseChild = { | ||
| on: vi.fn((event, callback) => { | ||
| if (event === 'close') { | ||
| setTimeout(() => callback(0), 2000) | ||
| } | ||
| }) | ||
| } | ||
| mockSpawn | ||
| .mockReturnValueOnce(immediateCloseChild as any) | ||
| .mockReturnValueOnce(immediateCloseChild as any) | ||
| .mockReturnValue(slowCloseChild as any) | ||
|
|
||
| await service.downloadUv('darwin', 'arm64', progressCallback) | ||
|
|
||
| expect(progressCallback).toHaveBeenCalled() | ||
| expect(mockLogger.error).not.toHaveBeenCalled() | ||
| expect(statusSequence).toContain('downloading') | ||
| expect(statusSequence).toContain('extracting') | ||
| expect(statusSequence).toContain('completed') | ||
| }) | ||
|
|
||
| it('should calculate download speed and remaining time', async () => { | ||
| const progressSnapshots: DownloadProgress[] = [] | ||
| const progressCallback = vi.fn((progress: DownloadProgress) => { | ||
| progressSnapshots.push({ ...progress }) | ||
| }) | ||
|
|
||
| const mockRequest = { on: vi.fn(), destroy: vi.fn() } | ||
| const mockResponse = new PassThrough() | ||
| ;(mockResponse as any).statusCode = 200 | ||
| ;(mockResponse as any).headers = { 'content-length': '20480' } | ||
|
|
||
| httpsGetSpy.mockImplementation((_url, _options, callback) => { | ||
| setTimeout(() => { | ||
| callback(mockResponse) | ||
| setTimeout(() => { | ||
| mockResponse.write(Buffer.alloc(10240)) | ||
| setTimeout(() => { | ||
| mockResponse.write(Buffer.alloc(10240)) | ||
| mockResponse.end() | ||
| }, 1100) | ||
| }, 10) | ||
| }, 10) | ||
| return mockRequest | ||
| }) | ||
|
|
||
| const immediateCloseChild = { | ||
| on: vi.fn((event, callback) => { | ||
| if (event === 'close') callback(0) | ||
| }) | ||
| } | ||
| mockSpawn | ||
| .mockReturnValueOnce(immediateCloseChild as any) | ||
| .mockReturnValueOnce(immediateCloseChild as any) | ||
| .mockReturnValue(immediateCloseChild as any) | ||
|
|
||
| await service.downloadUv('darwin', 'arm64', progressCallback) | ||
|
|
||
| expect(mockLogger.error).not.toHaveBeenCalled() | ||
| const progressWithSpeed = progressSnapshots.find((snapshot) => snapshot.speed > 0) | ||
| expect(progressWithSpeed).toBeDefined() | ||
| expect(progressWithSpeed!).toHaveProperty('remainingTime') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
进度/速度相关测试易受真实计时影响,建议使用假定时器减少抖动
这些用例依赖 setTimeout 和真实时间间隔,可能在 CI 上偶发失败。建议使用 vi.useFakeTimers()/vi.setSystemTime() 并在合适位置 vi.advanceTimersByTime() 驱动。
| this.process.on('exit', (code, signal) => { | ||
| logger.warn('Media Server 进程退出', { code, signal }) | ||
|
|
||
| const previousPort = this.port | ||
|
|
||
| this.status = 'stopped' | ||
| this.process = null | ||
| this.startTime = null | ||
| this.port = null | ||
|
|
||
| // 通知前端端口已失效 | ||
| if (previousPort !== null) { | ||
| this.notifyPortChanged(null) | ||
| } | ||
|
|
||
| // 如果是异常退出,尝试重启 | ||
| if (code !== 0 && this.restartAttempts < this.MAX_RESTART_ATTEMPTS) { | ||
| this.restartAttempts++ | ||
| logger.info('尝试重启 Media Server', { | ||
| attempt: this.restartAttempts, | ||
| maxAttempts: this.MAX_RESTART_ATTEMPTS | ||
| }) | ||
|
|
||
| setTimeout(() => { | ||
| this.start(finalConfig).catch((error) => { | ||
| logger.error('重启 Media Server 失败', { error }) | ||
| }) | ||
| }, 2000 * this.restartAttempts) // 递增延迟 | ||
| } else if (this.restartAttempts >= this.MAX_RESTART_ATTEMPTS) { | ||
| logger.error('Media Server 重启次数过多,停止重启') | ||
| this.lastError = '服务异常退出次数过多' | ||
| this.status = 'error' | ||
| } |
There was a problem hiding this comment.
退出即重启的逻辑缺少“意图判断”,会在手动停止或启动失败清理时误触发重启
当前依据 code !== 0 就重启:手动 stop()(SIGTERM)或启动失败后的 kill 都会进入该分支,引发隐性重启循环。
建议:
- 引入 shouldAutoRestart 标志,start 时开启,stop/启动失败清理时关闭;
- exit 回调中仅在 shouldAutoRestart 为 true 时执行重启逻辑。
示例修改(片段):
- // 如果是异常退出,尝试重启
- if (code !== 0 && this.restartAttempts < this.MAX_RESTART_ATTEMPTS) {
+ // 如果是异常退出且允许自动重启,尝试重启
+ if (this.shouldAutoRestart && code !== 0 && this.restartAttempts < this.MAX_RESTART_ATTEMPTS) {
this.restartAttempts++
...
- } else if (this.restartAttempts >= this.MAX_RESTART_ATTEMPTS) {
+ } else if (this.shouldAutoRestart && this.restartAttempts >= this.MAX_RESTART_ATTEMPTS) {
...
}需要在类中新增并管理该标志,见下方补充代码。
// 补充:在类字段处新增
private shouldAutoRestart = true
// 在 start() 成功 spawn 前后确保:
this.shouldAutoRestart = true
// 在 catch 分支的清理前
this.shouldAutoRestart = false
// 在 stop() 开始时
this.shouldAutoRestart = false🤖 Prompt for AI Agents
In src/main/services/MediaServerService.ts around lines 318-350, the exit
handler restarts the process whenever code !== 0 which causes unwanted
auto-restarts after intentional stop() or failed start cleanup; add a private
boolean field shouldAutoRestart to the class and use it to gate restart logic in
the exit callback (only attempt restart when shouldAutoRestart === true). Set
shouldAutoRestart = true right before/after a successful spawn in start(), set
shouldAutoRestart = false at the start of stop(), and set it false in any
start() failure cleanup path before killing/clearing the process; keep existing
restartAttempts/max logic but only execute it when the flag allows it.
| public async stop(): Promise<boolean> { | ||
| try { | ||
| if (this.status === 'stopped') { | ||
| logger.info('Media Server 已停止') | ||
| return true | ||
| } | ||
|
|
||
| if (!this.process) { | ||
| this.status = 'stopped' | ||
| return true | ||
| } | ||
|
|
||
| this.status = 'stopping' | ||
| logger.info('正在停止 Media Server', { pid: this.process.pid }) | ||
|
|
||
| // 停止健康检查 | ||
| this.stopHealthCheck() | ||
|
|
||
| // 优雅关闭 | ||
| return new Promise<boolean>((resolve) => { | ||
| if (!this.process) { | ||
| this.status = 'stopped' | ||
| resolve(true) | ||
| return | ||
| } | ||
|
|
||
| const pid = this.process.pid | ||
|
|
||
| // 设置超时强制终止 | ||
| const forceKillTimer = setTimeout(() => { | ||
| if (this.process && !this.process.killed) { | ||
| logger.warn('强制终止 Media Server', { pid }) | ||
| this.process.kill('SIGKILL') | ||
| } | ||
| }, 5000) // 5 秒后强制终止 | ||
|
|
||
| // 监听退出 | ||
| this.process.once('exit', () => { | ||
| clearTimeout(forceKillTimer) | ||
|
|
||
| const previousPort = this.port | ||
|
|
||
| this.status = 'stopped' | ||
| this.process = null | ||
| this.startTime = null | ||
| this.port = null | ||
|
|
||
| // 通知前端端口已失效 | ||
| if (previousPort !== null) { | ||
| this.notifyPortChanged(null) | ||
| } | ||
|
|
||
| logger.info('Media Server 已停止', { pid }) | ||
| resolve(true) | ||
| }) | ||
|
|
||
| // 发送 SIGTERM 信号 | ||
| this.process.kill('SIGTERM') | ||
| }) |
There was a problem hiding this comment.
stop() 时未抑制自动重启;需在发送 SIGTERM 之前关闭自动重启
为避免手动停止触发 exit 回调的重启逻辑,应先标记禁用:
this.status = 'stopping'
logger.info('正在停止 Media Server', { pid: this.process.pid })
+ // 防止手动停止触发自动重启
+ this.shouldAutoRestart = false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async stop(): Promise<boolean> { | |
| try { | |
| if (this.status === 'stopped') { | |
| logger.info('Media Server 已停止') | |
| return true | |
| } | |
| if (!this.process) { | |
| this.status = 'stopped' | |
| return true | |
| } | |
| this.status = 'stopping' | |
| logger.info('正在停止 Media Server', { pid: this.process.pid }) | |
| // 停止健康检查 | |
| this.stopHealthCheck() | |
| // 优雅关闭 | |
| return new Promise<boolean>((resolve) => { | |
| if (!this.process) { | |
| this.status = 'stopped' | |
| resolve(true) | |
| return | |
| } | |
| const pid = this.process.pid | |
| // 设置超时强制终止 | |
| const forceKillTimer = setTimeout(() => { | |
| if (this.process && !this.process.killed) { | |
| logger.warn('强制终止 Media Server', { pid }) | |
| this.process.kill('SIGKILL') | |
| } | |
| }, 5000) // 5 秒后强制终止 | |
| // 监听退出 | |
| this.process.once('exit', () => { | |
| clearTimeout(forceKillTimer) | |
| const previousPort = this.port | |
| this.status = 'stopped' | |
| this.process = null | |
| this.startTime = null | |
| this.port = null | |
| // 通知前端端口已失效 | |
| if (previousPort !== null) { | |
| this.notifyPortChanged(null) | |
| } | |
| logger.info('Media Server 已停止', { pid }) | |
| resolve(true) | |
| }) | |
| // 发送 SIGTERM 信号 | |
| this.process.kill('SIGTERM') | |
| }) | |
| this.status = 'stopping' | |
| logger.info('正在停止 Media Server', { pid: this.process.pid }) | |
| // 防止手动停止触发自动重启 | |
| this.shouldAutoRestart = false | |
| // 停止健康检查 | |
| this.stopHealthCheck() |
🤖 Prompt for AI Agents
In src/main/services/MediaServerService.ts around lines 402 to 460, the stop()
flow sends SIGTERM but does not disable the service's automatic-restart behavior
first; before sending SIGTERM, set the instance's auto-restart flag (e.g.
this.autoRestart = false) so the process exit handler will not trigger a
restart, and ensure the exit/restart logic elsewhere checks that flag before
performing any restart.
| private async executeCommand(command: string, args: string[], cwd?: string): Promise<string> { | ||
| return new Promise((resolve, reject) => { | ||
| const child = spawn(command, args, { | ||
| cwd, | ||
| stdio: 'pipe', | ||
| shell: process.platform === 'win32' | ||
| }) | ||
|
|
There was a problem hiding this comment.
Windows 下命令执行因 shell: true 与空格路径会直接失败
这里在 Windows 平台始终设置 shell: true,当 command 是带空格的绝对路径(常见于 C:\Users\John Doe\AppData\...)时,cmd.exe 会把空格前后的片段视作不同参数,导致所有 uv 相关命令执行失败,整条安装链路无法使用。请移除强制 shell,或至少在传入 shell 时对命令做适当引用处理,建议直接让 spawn 以默认 shell: false 运行可执行文件即可。
- const child = spawn(command, args, {
- cwd,
- stdio: 'pipe',
- shell: process.platform === 'win32'
- })
+ const child = spawn(command, args, {
+ cwd,
+ stdio: 'pipe',
+ shell: false
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private async executeCommand(command: string, args: string[], cwd?: string): Promise<string> { | |
| return new Promise((resolve, reject) => { | |
| const child = spawn(command, args, { | |
| cwd, | |
| stdio: 'pipe', | |
| shell: process.platform === 'win32' | |
| }) | |
| private async executeCommand(command: string, args: string[], cwd?: string): Promise<string> { | |
| return new Promise((resolve, reject) => { | |
| - const child = spawn(command, args, { | |
| - cwd, | |
| - stdio: 'pipe', | |
| - shell: process.platform === 'win32' | |
| const child = spawn(command, args, { | |
| cwd, | |
| stdio: 'pipe', | |
| shell: false | |
| }) | |
| // …rest of implementation… |
🤖 Prompt for AI Agents
In src/main/services/UvBootstrapperService.ts around lines 343 to 350, the spawn
call forces shell: true on Windows which breaks execution when the command is an
absolute path containing spaces; change the spawn invocation to avoid forcing
shell on Windows (use shell: false or omit shell) so the executable path is
passed directly, or if you must enable shell detect and properly quote/escape
the command and args before passing them to spawn; update the options to default
to shell: false and only set shell: true when running a shell string, ensuring
cwd/stdio remain correct and adjust call sites if they relied on shell behavior.
| import React, { useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react' | ||
| import styled, { css, keyframes } from 'styled-components' | ||
|
|
||
| import { | ||
| SettingDescription, | ||
| SettingDivider, | ||
| SettingGroup, | ||
| SettingRow, | ||
| SettingRowTitle, | ||
| SettingTitle | ||
| } from '.' | ||
|
|
||
| const logger = loggerService.withContext('MediaServerSection') | ||
|
|
||
| type DependencyType = 'ffmpeg' | 'ffprobe' | ||
|
|
||
| interface MediaServerSectionProps { | ||
| onDependencyReady?: (dependency: DependencyType) => void | ||
| triggerInstall?: boolean | ||
| onInstallTriggered?: () => void | ||
| } | ||
|
|
||
| const MediaServerSection = ({ | ||
| ref: forwardedRef, | ||
| ...props | ||
| }: MediaServerSectionProps & { ref?: React.RefObject<HTMLDivElement | null> }) => { | ||
| const { onDependencyReady, triggerInstall, onInstallTriggered } = props | ||
| const { theme } = useTheme() | ||
|
|
||
| const [serverInfo, setServerInfo] = useState<MediaServerInfo | null>(null) | ||
| const [venvInfo, setVenvInfo] = useState<PythonVenvInfo | null>(null) | ||
| const [isInstalling, setIsInstalling] = useState(false) | ||
| const [installProgress, setInstallProgressState] = useState<InstallProgress | null>(null) | ||
| const [showSuccessState, setShowSuccessState] = useState(false) | ||
| const [isHighlighted, setIsHighlighted] = useState(false) | ||
| const isCompletionHandledRef = useRef(false) | ||
| const successTimeoutRef = useRef<NodeJS.Timeout | null>(null) | ||
| const containerRef = useRef<HTMLDivElement>(null) | ||
|
|
||
| // 暴露 ref 给父组件 | ||
| useImperativeHandle(forwardedRef, () => containerRef.current as HTMLDivElement) |
There was a problem hiding this comment.
修复 forwardRef 使用错误,避免 ref 传递失效
这里通过解构 props 读取 ref,但函数组件不会收到 ref,父组件一旦尝试传递 ref 就会触发 “Function components cannot be given refs” 警告,useImperativeHandle 也永远不会对外暴露容器元素。请用 forwardRef 正式转发 ref,并同步调整导出。示例修复如下:
-import React, { useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react'
+import React, { forwardRef, useCallback, useEffect, useImperativeHandle, useRef, useState } from 'react'
…
-const MediaServerSection = ({
- ref: forwardedRef,
- ...props
-}: MediaServerSectionProps & { ref?: React.RefObject<HTMLDivElement | null> }) => {
- const { onDependencyReady, triggerInstall, onInstallTriggered } = props
+const MediaServerSection = forwardRef<HTMLDivElement, MediaServerSectionProps>((props, forwardedRef) => {
+ const { onDependencyReady, triggerInstall, onInstallTriggered } = props
…
- return (
+ return (
<HighlightWrapper ref={containerRef} $isHighlighted={isHighlighted}>
…
</HighlightWrapper>
)
-}
+})
MediaServerSection.displayName = 'MediaServerSection'
…
-export default MediaServerSection
+export default MediaServerSection同时在 useImperativeHandle 中直接转发 containerRef.current 即可。这样父组件就能安全地拿到 DOM 容器。
Also applies to: 516-653
🤖 Prompt for AI Agents
In src/renderer/src/pages/settings/MediaServerSection.tsx around lines 15-55,
the component incorrectly reads `ref` from props (function components don't
receive `ref`) which causes the "Function components cannot be given refs"
warning and prevents useImperativeHandle from exposing the DOM; fix by
converting the component to use React.forwardRef, accept (props, ref) instead of
destructuring `ref` from props, remove `ref` from props type, pass the forwarded
ref into useImperativeHandle (exposing containerRef.current), and update the
component export to the forwardRef wrapper; apply the same forwardRef refactor
to the other occurrence noted at lines 516-653.
| // 监听 Media Server 端口变更事件 | ||
| if (typeof window !== 'undefined' && window.electron?.ipcRenderer) { | ||
| window.electron.ipcRenderer.on( | ||
| IpcChannel.MediaServer_PortChanged, | ||
| (_event: any, newPort: number | null) => { | ||
| if (newPort === null) { | ||
| logger.info('收到 Media Server 停止通知,清空端口缓存') | ||
| } else { | ||
| logger.info('收到 Media Server 端口变更通知', { newPort }) | ||
| } | ||
| SessionService.resetPort() | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
类定义前引用 SessionService 会直接抛 ReferenceError
ipcRenderer.on(... => SessionService.resetPort()) 这段代码在类声明之前执行。由于 class 存在 TDZ,模块加载到这里时就会抛 ReferenceError: Cannot access 'SessionService' before initialization,渲染进程会在首次导入该模块时直接崩溃。请把监听逻辑移动到类定义之后(或改成静态初始化),确保 SessionService 已完成定义后再调用。
-// 监听 Media Server 端口变更事件
-if (typeof window !== 'undefined' && window.electron?.ipcRenderer) {
- window.electron.ipcRenderer.on(
- IpcChannel.MediaServer_PortChanged,
- (_event: any, newPort: number | null) => {
- if (newPort === null) {
- logger.info('收到 Media Server 停止通知,清空端口缓存')
- } else {
- logger.info('收到 Media Server 端口变更通知', { newPort })
- }
- SessionService.resetPort()
- }
- )
-}
-
export class SessionService {
…
}
+
+// 监听 Media Server 端口变更事件(需放在类定义之后)
+if (typeof window !== 'undefined' && window.electron?.ipcRenderer) {
+ window.electron.ipcRenderer.on(
+ IpcChannel.MediaServer_PortChanged,
+ (_event: any, newPort: number | null) => {
+ if (newPort === null) {
+ logger.info('收到 Media Server 停止通知,清空端口缓存')
+ } else {
+ logger.info('收到 Media Server 端口变更通知', { newPort })
+ }
+ SessionService.resetPort()
+ }
+ )
+}🤖 Prompt for AI Agents
In src/renderer/src/services/SessionService.ts around lines 6 to 19, the
ipcRenderer.on(...) handler references SessionService.resetPort before the class
is defined which triggers a ReferenceError due to TDZ; move the entire listener
registration so it runs after the SessionService class declaration (or implement
a static initialization method on SessionService and call that after the class
is defined) to ensure SessionService is initialized before calling resetPort,
and keep the handler logic identical when relocated.
Summary by CodeRabbit