chore: ffmpeg record log, remove legacy video fields, logging fallback

Co-authored-by: Cursor <cursoragent@cursor.com>

src/jiangchang_skill_core/rpa/video_session.py

@@ -1,7 +1,4 @@
-"""RPA 运行录屏会话：ffmpeg 桌面录制 → 字幕 + 背景音乐 → 最终 MP4。
-
-Playwright 仅用于浏览器自动化，不负责录屏。
-"""
+"""RPA 运行录屏会话：ffmpeg 桌面录制 → 字幕 + 背景音乐 → 最终 MP4。"""
 
 from __future__ import annotations
 
@@ -15,7 +12,7 @@ import time
 from dataclasses import dataclass
 from datetime import datetime
 from pathlib import Path
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, IO, List, Optional
 
 from .. import config
 
@@ -222,8 +219,6 @@ class RpaVideoSession:
         self.warnings: List[str] = []
         self.output_video_path: Optional[str] = None
         self.capture_path: Optional[str] = None
-        # 已废弃：勿用于 Playwright；始终为 None。
-        self.record_video_dir: Optional[str] = None
         self.artifacts: Dict[str, Any] = {}
 
         ts = datetime.now().strftime("%Y%m%d_%H%M%S")
@@ -236,6 +231,7 @@ class RpaVideoSession:
         )
         self._subtitles_dir = os.path.join(self._artifact_root, "subtitles")
         self._logs_dir = os.path.join(self._artifact_root, "logs")
+        self._ffmpeg_record_log_path = os.path.join(self._logs_dir, "ffmpeg-record.log")
         self._capture_path = os.path.join(self._artifact_root, "capture.mp4")
         self._srt_path = os.path.join(self._subtitles_dir, f"{self._video_stem}.srt")
         self._planned_output = os.path.join(self._videos_dir, f"{self._video_stem}.mp4")
@@ -243,6 +239,7 @@ class RpaVideoSession:
         self._steps: List[_StepEntry] = []
         self._t0: Optional[float] = None
         self._ffmpeg_proc: Optional[subprocess.Popen] = None
+        self._ffmpeg_record_log_file: Optional[IO[str]] = None
 
         if self.enabled:
             os.makedirs(self._videos_dir, exist_ok=True)
@@ -250,6 +247,21 @@ class RpaVideoSession:
             os.makedirs(self._logs_dir, exist_ok=True)
             os.makedirs(self._artifact_root, exist_ok=True)
 
+    def _record_log_hint(self) -> str:
+        if self.enabled and self._ffmpeg_record_log_path:
+            return f"; record_log={self._ffmpeg_record_log_path}"
+        return ""
+
+    def _append_warning(self, code: str, detail: str = "") -> None:
+        msg = code if not detail else f"{code}: {detail}"
+        if code in (
+            "ffmpeg_record_start_failed",
+            "ffmpeg_capture_missing",
+            "ffmpeg_compose_failed",
+        ):
+            msg += self._record_log_hint()
+        self.warnings.append(msg)
+
     def step(self, text: str, *, duration: float = 4.0) -> None:
         self.add_step(text, duration=duration)
 
@@ -262,6 +274,16 @@ class RpaVideoSession:
         elapsed = time.monotonic() - self._t0
         self._steps.append(_StepEntry(start=elapsed, text=text.strip(), duration=duration))
 
+    def _close_ffmpeg_record_log(self) -> None:
+        fh = self._ffmpeg_record_log_file
+        self._ffmpeg_record_log_file = None
+        if fh is None:
+            return
+        try:
+            fh.close()
+        except Exception:
+            pass
+
     def _start_ffmpeg_capture(self) -> None:
         if not self.enabled:
             return
@@ -271,26 +293,39 @@ class RpaVideoSession:
 
         cmd = _build_desktop_record_cmd(self._capture_path)
         if cmd is None:
-            self.warnings.append(
-                "ffmpeg_record_start_failed: desktop capture only supported on Windows"
+            self._append_warning(
+                "ffmpeg_record_start_failed",
+                "desktop capture only supported on Windows",
             )
             return
 
+        stderr_target: Any = subprocess.DEVNULL
+        try:
+            os.makedirs(self._logs_dir, exist_ok=True)
+            self._ffmpeg_record_log_file = open(
+                self._ffmpeg_record_log_path, "a", encoding="utf-8"
+            )
+            stderr_target = self._ffmpeg_record_log_file
+        except Exception as exc:
+            self.warnings.append(f"ffmpeg_record_log_open_failed: {exc}")
+
         try:
             self._ffmpeg_proc = subprocess.Popen(
                 cmd,
                 stdin=subprocess.PIPE,
                 stdout=subprocess.DEVNULL,
-                stderr=subprocess.DEVNULL,
+                stderr=stderr_target,
             )
             self.capture_path = self._capture_path
         except Exception as exc:
-            self.warnings.append(f"ffmpeg_record_start_failed: {exc}")
+            self._append_warning("ffmpeg_record_start_failed", str(exc))
             self._ffmpeg_proc = None
+            self._close_ffmpeg_record_log()
 
     def _stop_ffmpeg_capture(self) -> None:
         proc = self._ffmpeg_proc
         if proc is None:
+            self._close_ffmpeg_record_log()
             return
         if proc.poll() is None:
             try:
@@ -314,8 +349,8 @@ class RpaVideoSession:
                 except Exception:
                     pass
         self._ffmpeg_proc = None
-        await_sleep = 0.3
-        time.sleep(await_sleep)
+        self._close_ffmpeg_record_log()
+        time.sleep(0.3)
 
     async def __aenter__(self) -> "RpaVideoSession":
         if self.enabled:
@@ -342,7 +377,7 @@ class RpaVideoSession:
         try:
             capture = Path(self._capture_path)
             if not capture.is_file() or capture.stat().st_size <= 0:
-                self.warnings.append("ffmpeg_capture_missing")
+                self._append_warning("ffmpeg_capture_missing")
                 self.artifacts = self.summary()
                 return
 
@@ -396,7 +431,7 @@ class RpaVideoSession:
                     ]
                 )
             if not ok:
-                self.warnings.append(f"ffmpeg_compose_failed: {err}")
+                self._append_warning("ffmpeg_compose_failed", err)
                 self.artifacts = self.summary()
                 return
 
@@ -407,10 +442,16 @@ class RpaVideoSession:
         self.artifacts = self.summary()
 
     def summary(self) -> Dict[str, Any]:
+        record_log = (
+            os.path.abspath(self._ffmpeg_record_log_path)
+            if self.enabled and self._ffmpeg_record_log_path
+            else None
+        )
         return {
             "enabled": self.enabled,
             "path": self.output_video_path,
             "capture_path": self.capture_path,
+            "record_log_path": record_log,
             "warnings": list(self.warnings),
         }
 

src/jiangchang_skill_core/unified_logging.py

@@ -20,7 +20,10 @@ _logger_name: str = ""
 
 def get_unified_logs_dir() -> str:
     path = os.path.join(get_data_root(), get_user_id(), "logs")
-    os.makedirs(path, exist_ok=True)
+    try:
+        os.makedirs(path, exist_ok=True)
+    except OSError:
+        pass
     return path
 
 
@@ -30,7 +33,10 @@ def get_skill_log_file_path() -> str:
     if override:
         parent = os.path.dirname(os.path.abspath(override))
         if parent:
-            os.makedirs(parent, exist_ok=True)
+            try:
+                os.makedirs(parent, exist_ok=True)
+            except OSError:
+                pass
         return os.path.abspath(override)
     return os.path.join(get_unified_logs_dir(), "jiangchang.log")
 
@@ -81,10 +87,16 @@ def _backup_count() -> int:
         return 30
 
 
+def _attach_fallback_handler(log: logging.Logger, fmt: logging.Formatter) -> None:
+    """文件日志不可用时仅挂内存 handler，不向 stderr 打印 logging 异常。"""
+    log.addHandler(logging.NullHandler())
+
+
 def setup_skill_logging(skill_slug: str, logger_name: str) -> None:
     """
     幂等：为指定 logger 挂载统一文件日志与可选 stderr。
     须在进程早期调用（如 CLI main 在业务日志之前）。
+    文件日志目录无权限或不可写时降级为 no-op，不影响 CLI 返回码。
     """
     global _skill_slug, _logger_name
     ensure_trace_for_process()
@@ -95,30 +107,41 @@ def setup_skill_logging(skill_slug: str, logger_name: str) -> None:
     if log.handlers:
         return
     log.setLevel(_log_level_from_env())
-    path = get_skill_log_file_path()
-    fh = TimedRotatingFileHandler(
-        path,
-        when="midnight",
-        interval=1,
-        backupCount=_backup_count(),
-        encoding="utf-8",
-        delay=True,
-    )
     fmt = logging.Formatter(_FORMAT, datefmt=_DATEFMT)
-    fh.setFormatter(fmt)
-    fh.addFilter(_SkillContextFilter())
-    log.addHandler(fh)
+    filt = _SkillContextFilter()
+
+    try:
+        path = get_skill_log_file_path()
+        fh = TimedRotatingFileHandler(
+            path,
+            when="midnight",
+            interval=1,
+            backupCount=_backup_count(),
+            encoding="utf-8",
+            delay=True,
+        )
+        fh.setFormatter(fmt)
+        fh.addFilter(filt)
+        log.addHandler(fh)
+    except (OSError, PermissionError, ValueError):
+        _attach_fallback_handler(log, fmt)
+    except Exception:
+        _attach_fallback_handler(log, fmt)
+
     if (os.getenv("JIANGCHANG_LOG_TO_STDERR") or "").strip().lower() in (
         "1",
         "true",
         "yes",
         "on",
     ):
-        sh = logging.StreamHandler(sys.stderr)
-        sh.setLevel(logging.WARNING)
-        sh.setFormatter(fmt)
-        sh.addFilter(_SkillContextFilter())
-        log.addHandler(sh)
+        try:
+            sh = logging.StreamHandler(sys.stderr)
+            sh.setLevel(logging.WARNING)
+            sh.setFormatter(fmt)
+            sh.addFilter(filt)
+            log.addHandler(sh)
+        except Exception:
+            pass
     log.propagate = False
 
 
@@ -144,13 +167,18 @@ def attach_unified_file_handler(
     lg = logging.getLogger(logger_name)
     lg.handlers.clear()
     lg.setLevel(level)
-    parent = os.path.dirname(os.path.abspath(log_path))
-    if parent:
-        os.makedirs(parent, exist_ok=True)
-    fh = logging.FileHandler(log_path, encoding="utf-8")
     fmt = logging.Formatter(_FORMAT, datefmt=_DATEFMT)
-    fh.setFormatter(fmt)
-    fh.addFilter(_SkillContextFilter())
-    lg.addHandler(fh)
+    try:
+        parent = os.path.dirname(os.path.abspath(log_path))
+        if parent:
+            os.makedirs(parent, exist_ok=True)
+        fh = logging.FileHandler(log_path, encoding="utf-8")
+        fh.setFormatter(fmt)
+        fh.addFilter(_SkillContextFilter())
+        lg.addHandler(fh)
+    except (OSError, PermissionError, ValueError):
+        lg.addHandler(logging.NullHandler())
+    except Exception:
+        lg.addHandler(logging.NullHandler())
     lg.propagate = False
     return lg

tests/test_video_session.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-"""RpaVideoSession：ffmpeg 桌面录屏，无 Playwright record_video_dir。"""
+"""RpaVideoSession：ffmpeg 桌面录屏。"""
 from __future__ import annotations
 
 import asyncio
@@ -28,18 +28,17 @@ class TestRpaVideoSession(unittest.TestCase):
                     batch_id="batch_test",
                 )
                 self.assertFalse(session.enabled)
-                self.assertIsNone(session.record_video_dir)
-                self.assertIsNone(session.output_video_path)
-                self.assertIsNone(session.capture_path)
                 summ = session.summary()
                 self.assertFalse(summ["enabled"])
                 self.assertIsNone(summ["path"])
+                self.assertIsNone(summ["capture_path"])
+                self.assertIsNone(summ["record_log_path"])
         finally:
             if old is not None:
                 os.environ["OPENCLAW_RECORD_VIDEO"] = old
             config.reset_cache()
 
-    def test_enabled_paths_no_raw_video(self) -> None:
+    def test_enabled_paths_use_ffmpeg_capture(self) -> None:
         os.environ["OPENCLAW_RECORD_VIDEO"] = "1"
         try:
             with tempfile.TemporaryDirectory() as tmp:
@@ -51,12 +50,16 @@ class TestRpaVideoSession(unittest.TestCase):
                     title="接收订单",
                 )
                 self.assertTrue(session.enabled)
-                self.assertIsNone(session.record_video_dir)
                 self.assertTrue(
                     session._capture_path.replace("\\", "/").endswith(
                         "/rpa-artifacts/exmail_20260602_120000/capture.mp4"
                     )
                 )
+                self.assertTrue(
+                    session._ffmpeg_record_log_path.replace("\\", "/").endswith(
+                        "/rpa-artifacts/exmail_20260602_120000/logs/ffmpeg-record.log"
+                    )
+                )
                 self.assertTrue(
                     session._planned_output.replace("\\", "/").startswith(
                         tmp.replace("\\", "/") + "/videos/"
@@ -87,15 +90,17 @@ class TestRpaVideoSession(unittest.TestCase):
                         batch_id="batch_ff",
                     ) as video:
                         video.add_step("测试步骤")
-                        with patch.object(video, "finalize", return_value=None):
-                            pass
 
-                asyncio.run(_run())
+                with patch.object(RpaVideoSession, "finalize", return_value=None):
+                    asyncio.run(_run())
+
                 self.assertTrue(mock_popen.called)
                 cmd = mock_popen.call_args[0][0]
                 self.assertIn("ffmpeg", cmd)
                 self.assertIn("gdigrab", cmd)
                 self.assertIn("desktop", cmd)
+                kwargs = mock_popen.call_args[1]
+                self.assertIsNot(kwargs.get("stderr"), os.devnull)
         finally:
             os.environ.pop("OPENCLAW_RECORD_VIDEO", None)
             config.reset_cache()
@@ -106,16 +111,6 @@ class TestRpaVideoSession(unittest.TestCase):
         try:
             with tempfile.TemporaryDirectory() as tmp:
                 config.reset_cache()
-
-                async def _run() -> None:
-                    async with RpaVideoSession(
-                        skill_slug="receive-order",
-                        skill_data_dir=tmp,
-                        batch_id="batch_no_ff",
-                    ):
-                        pass
-
-                asyncio.run(_run())
                 session = RpaVideoSession(
                     skill_slug="receive-order",
                     skill_data_dir=tmp,
@@ -138,10 +133,11 @@ class TestRpaVideoSession(unittest.TestCase):
                     batch_id="batch_warn",
                 )
                 asyncio.run(session.finalize())
-                self.assertIn("ffmpeg_capture_missing", session.warnings)
+                self.assertTrue(
+                    any("ffmpeg_capture_missing" in w for w in session.warnings)
+                )
+                self.assertIn("record_log=", "".join(session.warnings))
                 self.assertIsNone(session.output_video_path)
-                summ = session.summary()
-                self.assertIsNone(summ["path"])
         finally:
             os.environ.pop("OPENCLAW_RECORD_VIDEO", None)
             config.reset_cache()