fix: 防止任务删除误删源视频目录并跟踪审查问题

docs/代码审查问题跟踪.md

@@ -0,0 +1,55 @@
+# 代码审查问题跟踪
+
+本文件跟踪本次项目审查发现的问题、修复范围与验证结果。状态：待处理 / 处理中 / 已修复。只有完成实现与相关验证后才标记已修复；代码修改不代表已部署。
+
+全部审查问题的修复统一在 `fix/review-improvements` 分支推进，验证完成后由用户合并到 `master`。
+
+## 缺陷清单
+
+| ID | 优先级 | 问题 | 状态 | 验收标准 |
+| --- | --- | --- | --- | --- |
+| R01 | P0 | 普通删除接口可能删除批量源视频目录；清理逻辑从输入路径推导删除范围 | 已修复 | 普通接口拒绝单独删除批量 run；手动与自动清理只删除该上传任务的私有目录；视频、旁挂字幕及其他任务文件不受影响 |
+| R02 | P1 | 自适应线程池限流后可能扩容；退出标记位于积压队列尾部，缩容不及时 | 待处理 | 限流不增加并发；降低目标后不再超额提交；真实积压队列验证 |
+| R03 | P1 | 最后节点执行时暂停再继续会覆盖有效下载 URI；批量缺产物仍清理并完成 | 待处理 | 收尾幂等，恢复后全部必需产物可下载；缺产物不清理工作空间 |
+| R04 | P1 | 环形 DAG 可通过保存校验，执行失败后仍在 QUEUED 堵塞队列 | 待处理 | 保存/发布拒绝无效 DAG；历史无效任务进入 FAILED，不阻塞后续任务 |
+| R05 | P1 | 翻译按固定四行解析 SRT；补齐行数不能保证文本与时间轴对应 | 待处理 | 合法多行 cue 正确解析；按稳定 ID 回填译文并校验缺失、重复项 |
+| R06 | P1 | OCR 跨空白帧合并相同字幕；临时请求失败被永久存为无文字 | 待处理 | 空白帧结束当前字幕段；无文字和可重试失败分开存档 |
+| R07 | P2 | 抽帧/分块目录残留污染重跑；OCR/过滤存档无输入和参数指纹 | 待处理 | 输出减少时无旧文件混入；输入/有效参数变化时断点失效 |
+| R08 | P2 | 批量任务未固定工作流版本，恢复收尾可能使用新版本定义；创建任务与明细不原子 | 待处理 | 执行与收尾使用固定版本；工作线程只看到完整的已创建任务 |
+
+## 优化与维护清单
+
+下列项目为优化建议，收益需按真实负载验证，尚未承诺实施顺序。
+
+| ID | 项目 | 状态 | 验收方向 |
+| --- | --- | --- | --- |
+| O01 | Whisper 模型缓存及上传/批量共享 GPU 并发额度 | 待处理 | 相同配置复用模型；初始化线程安全；显存有界 |
+| O02 | Whisper 分块、LLM 翻译批次断点 | 待处理 | 中断后只补未完成部分；输入与参数身份匹配 |
+| O03 | VAD 流式读取、向量化 RMS | 待处理 | 与现有 RMS 结果一致；内存不随全片样本数增长；记录性能对比 |
+| O04 | LLM 公共客户端、连接复用、统一限流重试 | 待处理 | 网络故障处理一致；提示词保留在各节点；避免重复计费与无界重试 |
+| O05 | 字幕纠错批量化、减少重复上下文生成 | 待处理 | 保留目标 ID；固定输入上下文；真实字幕质量验证 |
+| O06 | 合并媒体探测、减少 OCR 小文件，评估分段抽帧与 OCR 重叠执行 | 待处理 | 保持时间轴和落盘协议，测量 I/O 与吞吐变化 |
+| O07 | 统一 SRT 解析/序列化/时间戳，移除生产代码对 tests 包的依赖 | 待处理 | 正式安装包可运行；多行 cue、空字幕、时间精度回归通过 |
+| O08 | 统一运行状态、工作空间与 JSONL 存档公共能力 | 待处理 | 状态条件更新、文件归属明确、存档损坏可恢复 |
+| O09 | 后台线程停止与窗口统计可靠性 | 待处理 | 停止等待可中断；不丢弃仍存活线程；并发统计一致 |
+| O10 | 批量目录索引、聚合查询、数据库索引与轮询 | 待处理 | 避免每视频重复扫描目录；GET 不刷新状态更新时间；相关查询有界 |
+| O11 | Whisper 解码耗时统计 | 待处理 | 消费 segments 生成器后计时，使用实际块时长计算实时倍率 |
+| O12 | 校正测试断言与文档漂移 | 待处理 | 成功验证包括产物可读/可下载；对齐测试可在正确结果下通过；文档与模型/断点实现一致 |
+
+## R01 修复记录
+
+- 根因：普通任务删除接口未区分 `source=batch`，直接递归删除 `Path(input_uri).parent`；自动清理器也使用输入路径推导删除目录。
+- 处理约定：普通删除接口拒绝批量 run（422），提示通过批量任务入口删除；上传任务只清理 `<storage>/uploads/<run_id>` 和 `<storage>/runs/<run_id>`。批量任务已有专用删除入口，保留源视频和视频旁成品。
+- 测试要求：使用 `testdata/` 真实视频与字幕副本、临时数据库及私有存储；覆盖批量任务拒绝删除、正常上传清理、历史外部输入路径保护和自动清理路径保护。
+- 实现：[普通删除接口](../src/wov_app/routers/apps.py) 在删除前拒绝批量 run；[自动清理器](../src/wov_app/maintenance.py) 和普通接口均按私有目录布局定位清理范围。
+- 回归测试：[test_run_deletion_safety.py](../tests/test_run_deletion_safety.py)，覆盖五种状态的批量 run、外部输入路径与旁挂字幕保护、其他任务目录保护。
+- TDD 红：`uv run pytest tests/test_run_deletion_safety.py -q --tb=short`，7 failed；批量 run 被错误删除，两个外部路径用例复现媒体文件丢失。
+- TDD 绿及相关回归：`uv run pytest tests/test_run_deletion_safety.py tests/test_apps_api.py tests/test_batch.py tests/test_maintenance.py -q`，73 passed，7.73 秒；仅有既存 Starlette/httpx 弃用警告。
+- 状态：修复及验证完成，纳入 `fix/review-improvements` 分支；未部署。运行中的旧进程需加载新代码后才能获得保护。
+- 后续边界：普通列表仍会显示批量 run，但点击删除会收到批量入口提示；运行中删除的统一取消/状态协调属于 O08，未在本次展开。
+
+## 审查基线
+
+- 修复前全套测试：`uv run pytest`，369 passed、6 skipped，76.75 秒。
+- 隔离复现已确认：源视频目录误删、限流后 1 → 19 并发、队列积压时缩容滞后、暂停恢复后成品 URI 失效、环形 DAG 阻塞队首、多行 SRT 损坏、OCR 跨空白合并、重用抽帧目录留下旧尾帧。
+- 本文件中的“已修复”只表示当前工作区实现及验证完成；部署状态需另行记录。

src/wov_app/maintenance.py

@@ -82,9 +82,9 @@ class OrphanCleaner:
             run = self.db.get_run(run_id)
             if run is None:
                 continue
-            # 批量处理运行（source=batch）跳过清理：其产物在用户视频旁的同名
+            # 批量处理运行（source=batch）跳过清理：其工作空间位于独立的
-            # 文件夹里，不在主存储目录下；_has_files 检查不到会误判为孤儿删除，
+            # storage/batch 层级，_has_files 检查主 runs 目录会误判为孤儿。
-            # 且 _remove_run 还会删除 input_uri 的父目录（用户的视频文件夹）。
+            # 批量记录与私有工作空间由批量引擎负责收尾，用户媒体目录始终保留。
             if run.get("source") == "batch":
                 continue
             if run["status"] != "COMPLETED":
@@ -125,10 +125,10 @@ class OrphanCleaner:
         return any(path.is_file() for path in run_dir.rglob("*"))
 
     def _remove_run(self, run_id: str, run: dict) -> int:
-        """删除孤儿任务：数据库记录（含产物）、上传目录与步骤目录。"""
+        """删除孤儿上传任务的记录与私有目录，保留任何外部输入路径。"""
         self.db.delete_run(run_id)
-        input_uri = run.get("input_uri")
+        # 与手动删除一致，只按私有存储布局定位；历史记录的 input_uri
-        if input_uri:
+        # 可能引用用户媒体库，不能把其父目录当作可递归删除的上传目录。
-            shutil.rmtree(Path(input_uri).parent, ignore_errors=True)
+        shutil.rmtree(self.storage_dir / "uploads" / run_id, ignore_errors=True)
         shutil.rmtree(self.storage_dir / "runs" / run_id, ignore_errors=True)
         return 1

src/wov_app/routers/apps.py

@@ -184,18 +184,21 @@ def resume_run(run_id: str, db: Database = Depends(_get_db)) -> dict:
 
 @router.delete("/api/runs/{run_id}")
 def delete_run(run_id: str, db: Database = Depends(_get_db)) -> dict:
-    """删除任务：清理产物记录、上传文件与步骤产物目录。"""
+    """删除上传任务及其私有文件；批量 run 必须通过批量任务入口删除。"""
     run = db.get_run(run_id)
     if run is None:
         raise HTTPException(status_code=404, detail="run not found")
+    # 批量 run 的输入是用户原始视频，且由 batch_videos 关联管理。
+    # 在任何数据库/文件删除之前拒绝，避免误删媒体库或留下悬空的批量明细。
+    if run.get("source") == "batch":
+        raise HTTPException(status_code=422, detail="请通过批量任务入口删除该任务")
     from wov_app.config import STORAGE_DIR
 
     # 先删数据库记录（含产物表），再清理磁盘上的上传与中间产物。
     db.delete_run(run_id)
-    input_uri = run.get("input_uri")
+    # 删除范围仅来自该任务的私有存储布局，绝不由 input_uri 推导：
-    if input_uri:
+    # 即使历史上传记录引用外部路径，也必须保留源视频和同目录的用户文件。
-        # 上传文件位于 <storage>/uploads/<run_id>/，整目录一并删除。
+    shutil.rmtree(STORAGE_DIR / "uploads" / run_id, ignore_errors=True)
-        shutil.rmtree(Path(input_uri).parent, ignore_errors=True)
     # 步骤产物位于 <storage>/runs/<run_id>/，整目录一并删除。
     shutil.rmtree(STORAGE_DIR / "runs" / run_id, ignore_errors=True)
     return {"deleted": run_id}

tests/test_run_deletion_safety.py

@@ -0,0 +1,104 @@
+"""任务删除安全回归：真实媒体副本验证用户目录与任务私有目录的归属边界。"""
+
+import shutil
+from pathlib import Path
+
+import pytest
+from fastapi.testclient import TestClient
+
+from wov_app.config import STORAGE_DIR
+from wov_app.main import app
+from wov_app.db import Database
+from wov_app.maintenance import OrphanCleaner
+
+
+@pytest.fixture
+def media_library(tmp_path):
+    """复制已有真实视频与字幕，任何删除只作用于测试临时目录。"""
+    assets = Path(__file__).resolve().parent.parent / "testdata"
+    sources = [assets / "subtitle_10s.mp4", assets / "ocr_srt_run_ac7f480a3ccb.srt"]
+    if not all(source.is_file() for source in sources):
+        pytest.skip("缺少真实视频或字幕测试素材")
+    library = tmp_path / "media"
+    library.mkdir()
+    files = [library / "movie.mp4", library / "movie.CN.srt"]
+    for source, target in zip(sources, files):
+        shutil.copy2(source, target)
+    return files, [path.read_bytes() for path in files]
+
+
+def _create_run(db, run_id, video, source, status):
+    """在真实数据库登记任务，不启动模型或调度器。"""
+    workflow_id = "delete-safety"
+    db.upsert_workflow({"id": workflow_id, "name": "删除安全回归"})
+    db.create_run({
+        "id": run_id, "workflow_id": workflow_id, "workflow_version": 1,
+        "source": source, "status": status, "input_uri": str(video),
+        "created_at": "2020-01-01T00:00:00+00:00",
+        "updated_at": "2020-01-01T00:00:00+00:00",
+    })
+    # 登记可读的真实字幕产物，拒绝删除时应连同记录保留。
+    db.create_artifact({
+        "run_id": run_id, "node_id": "ass", "name": "subtitle",
+        "uri": str(video.with_suffix(".CN.srt")), "mime_type": "application/x-subrip",
+    })
+
+
+@pytest.mark.parametrize("status", ["QUEUED", "RUNNING", "PAUSED", "FAILED", "COMPLETED"])
+def test_normal_delete_rejects_batch_run(media_library, status):
+    """普通 DELETE 对所有状态的批量 run 返回 422，保留媒体与关联数据。"""
+    files, contents = media_library
+    run_id = f"run_batch_delete_safety_{status.lower()}"
+    with TestClient(app) as client:
+        db = app.state.db
+        _create_run(db, run_id, files[0], "batch", status)
+        before = db.get_run(run_id)
+        artifacts = db.list_artifacts(run_id)
+        response = client.delete(f"/api/runs/{run_id}")
+        assert response.status_code == 422
+        assert "批量任务" in response.json()["detail"]
+        assert db.get_run(run_id) == before
+        assert db.list_artifacts(run_id) == artifacts
+        assert [path.read_bytes() for path in files] == contents
+
+
+def test_upload_delete_ignores_external_input_parent(media_library):
+    """历史上传记录即使指向外部视频，也仅清理按 run_id 定位的私有目录。"""
+    files, contents = media_library
+    run_id = "run_external_delete_safety"
+    with TestClient(app) as client:
+        db = app.state.db
+        _create_run(db, run_id, files[0], "upload", "FAILED")
+        private_dirs = [STORAGE_DIR / kind / run_id for kind in ("uploads", "runs")]
+        for directory in private_dirs:
+            directory.mkdir(parents=True)
+            shutil.copy2(files[1], directory / "subtitle.srt")
+        # 同级其他任务目录也不能被扩大范围删除。
+        sibling = STORAGE_DIR / "uploads" / "run_neighbor_delete_safety"
+        sibling.mkdir(parents=True)
+        shutil.copy2(files[1], sibling / "subtitle.srt")
+        response = client.delete(f"/api/runs/{run_id}")
+        assert response.status_code == 200
+        assert db.get_run(run_id) is None
+        assert db.list_artifacts(run_id) == []
+        assert [path.read_bytes() for path in files] == contents
+        assert all(not directory.exists() for directory in private_dirs)
+        assert (sibling / "subtitle.srt").read_bytes() == contents[1]
+
+
+def test_orphan_cleanup_ignores_external_input_parent(media_library, tmp_path):
+    """自动清理过期上传任务时也不能从 input_uri 推导递归删除范围。"""
+    files, contents = media_library
+    db = Database(tmp_path / "orphan.db")
+    run_id = "run_orphan_delete_safety"
+    _create_run(db, run_id, files[0], "upload", "COMPLETED")
+    # 产物确实丢失，符合孤儿清理条件；源视频与旁挂字幕仍是用户数据。
+    db.delete_run_artifacts(run_id)
+    storage = tmp_path / "private"
+    upload_dir = storage / "uploads" / run_id
+    upload_dir.mkdir(parents=True)
+    shutil.copy2(files[0], upload_dir / "movie.mp4")
+    OrphanCleaner(db, storage).clean_once()
+    assert db.get_run(run_id) is None
+    assert [path.read_bytes() for path in files] == contents
+    assert not upload_dir.exists()