From a49c8b3b1507b8f79c67c963d6a62d0a30d47dbd Mon Sep 17 00:00:00 2001
From: kevin9327 <5299031+kevin9327@users.noreply.github.com>
Date: Wed, 9 Sep 2026 07:20:16 +0900
Subject: [PATCH] =?UTF-8?q?fix(factors):=20=E5=88=A0=E9=99=A4=E5=9B=A0?=
 =?UTF-8?q?=E5=AD=90=E8=A2=AB=E6=8B=92=E6=97=B6=E5=AE=9A=E4=B9=89=E5=B7=B2?=
 =?UTF-8?q?=E8=A2=AB=E5=88=A0=E6=8E=89?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

DELETE /api/factors/custom/{id} 的 404 守卫把 store.delete_one 当成存在性
探测, 但它会真的 unlink 文件并返回 True。当定义在盘上却没进注册表时
(load_into_registry 对注册失败的定义只告警跳过, 如复合因子的成员已被强制
删除), get_factor 为 None, 短路求值会走到 delete_one, 定义随即被删除。

随后的引用检查再返回 409「该因子仍有引用, 拒绝删除」—— 接口声称拒绝, 定义
却已经没了; 用户改带 force=true 重试只会得到 404, 定义无法恢复。

把 fail-closed 的引用检查移到存在性判定之前, 保证任何拒绝路径都不改磁盘。
---
 backend/app/api/factors.py                    |  6 +-
 .../tests/test_factor_delete_fail_closed.py   | 80 +++++++++++++++++++
 2 files changed, 84 insertions(+), 2 deletions(-)
 create mode 100644 backend/tests/test_factor_delete_fail_closed.py

diff --git a/backend/app/api/factors.py b/backend/app/api/factors.py
index 0879732..c8ca012 100644
--- a/backend/app/api/factors.py
+++ b/backend/app/api/factors.py
@@ -381,14 +381,16 @@ def delete_custom_factor(factor_id: str, request: Request, force: bool = Query(d
     data_dir = _data_dir(request)
     from app.factors.registry import get_factor
 
-    if get_factor(factor_id) is None and not store.delete_one(data_dir, factor_id):
-        raise HTTPException(status_code=404, detail=f"因子不存在: {factor_id}")
+    # 引用检查必须排在存在性判定之前: 下面用来探测「盘上是否有定义」的
+    # store.delete_one 本身就会删文件, 反过来会出现「拒绝删除」但定义已被删掉。
     references = _find_references(data_dir, factor_id)
     if references and not force:
         raise HTTPException(
             status_code=409,
             detail={"message": "该因子仍有引用, 拒绝删除 (可带 force=true 强制)", "references": references},
         )
+    if get_factor(factor_id) is None and not store.delete_one(data_dir, factor_id):
+        raise HTTPException(status_code=404, detail=f"因子不存在: {factor_id}")
     try:
         unregister_factor(factor_id)
     except ValueError as exc:
diff --git a/backend/tests/test_factor_delete_fail_closed.py b/backend/tests/test_factor_delete_fail_closed.py
new file mode 100644
index 0000000..c11d977
--- /dev/null
+++ b/backend/tests/test_factor_delete_fail_closed.py
@@ -0,0 +1,80 @@
+"""删除自定义因子: 有引用时必须 fail-closed —— 拒绝的同时定义文件不能已经被删掉。
+
+触发路径: 定义在磁盘上但没进注册表 (load_into_registry 对注册失败的定义只告警跳过,
+如复合因子的成员已被强制删除)。此时 get_factor() 为 None, 端点的 404 守卫会走到
+第二个分支, 而那个分支本身就会把文件删掉。
+"""
+from __future__ import annotations
+
+import json
+from pathlib import Path
+from types import SimpleNamespace
+
+import pytest
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+
+from app.api.factors import router
+
+FACTOR_ID = "uf_orphan_delete_guard"
+
+
+@pytest.fixture()
+def env(tmp_path):
+    """磁盘上有一个未注册的自定义因子, 且被一个策略引用。"""
+    factor_dir = tmp_path / "user_data" / "custom_factors"
+    factor_dir.mkdir(parents=True)
+    factor_path = factor_dir / f"{FACTOR_ID}.json"
+    factor_path.write_text(
+        json.dumps({
+            "id": FACTOR_ID,
+            "kind": "custom",
+            "label": "孤儿因子",
+            "formula": "rank(-ts_sum(change_pct, 5))",
+            "version": 1,
+            "status": "draft",
+        }, ensure_ascii=False),
+        encoding="utf-8",
+    )
+    strategies_dir = tmp_path / "strategies"
+    strategies_dir.mkdir(parents=True)
+    (strategies_dir / "s1.json").write_text(
+        json.dumps({"id": "s1", "factors": [FACTOR_ID]}, ensure_ascii=False),
+        encoding="utf-8",
+    )
+
+    app = FastAPI()
+    app.include_router(router)
+    app.state.repo = SimpleNamespace(store=SimpleNamespace(data_dir=Path(tmp_path)))
+    return TestClient(app), factor_path
+
+
+def test_delete_with_references_keeps_definition(env):
+    """409 拒绝删除时定义必须还在盘上, 否则用户既看不到因子也无法恢复。"""
+    client, factor_path = env
+
+    response = client.delete(f"/api/factors/custom/{FACTOR_ID}")
+
+    assert response.status_code == 409
+    assert response.json()["detail"]["references"] == ["strategies/s1.json"]
+    assert factor_path.exists()
+
+
+def test_delete_with_force_removes_definition(env):
+    """force=true 仍按原语义强制删除。"""
+    client, factor_path = env
+
+    response = client.delete(f"/api/factors/custom/{FACTOR_ID}?force=true")
+
+    assert response.status_code == 200
+    assert response.json()["removed_references"] == ["strategies/s1.json"]
+    assert not factor_path.exists()
+
+
+def test_delete_missing_factor_returns_404(env):
+    """不存在的因子仍返回 404。"""
+    client, _ = env
+
+    response = client.delete("/api/factors/custom/uf_not_there_at_all")
+
+    assert response.status_code == 404