fix(skills): answer invalid custom-skill history names with 400 (#7123)

* fix(skills): answer invalid custom-skill history names with 400

GET /api/skills/custom/{skill_name}/history let the ValueError from
validate_skill_name fall through to its catch-all, so a malformed name
returned 500 and logged a traceback on every request, while delete and
rollback answer 400.

Validate the name up front instead of adding a handler-wide
except ValueError: a corrupt history line raises JSONDecodeError, also a
ValueError, and must stay a server error. Both outcomes are pinned.

* fix(skills): keep corrupt history a 500 on rollback, validate delete names early

Sibling sweep of the history fix. Rollback's handler-wide except ValueError
also caught json.JSONDecodeError, so a corrupt history record answered 400
with the parser message and no log; it is now a logged 500 like history.

Delete only failed an invalid name inside the drained mutation tail, after
taking the owner's mutation lock, and logged it as a tail error although
_drain_skill_mutation keeps caller 4xx unlogged. It now rejects the name
before any storage work. Each guard is pinned by a mutation-checked test.

* docs(changelog): reference #7123 for the skill history status fix

* fix(skills): treat non-UTF-8 skill history as a server fault on rollback

UnicodeDecodeError is a ValueError too, so a non-UTF-8 history file still
reached rollback's ValueError -> 400 mapping while GET .../history answered
the same file with 500. Catch it alongside JSONDecodeError and pin both
corruption shapes on both routes.

Also drop the whitespace-only separator lines the changelog merge added
between other entries.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

CHANGELOG.md

@@ -1265,6 +1265,14 @@ This release closes that milestone with **621 merged pull requests**.
 
 ### Fixed
 
+- **skills:** `GET /api/skills/custom/{skill_name}/history` now answers an
+  invalid skill name (not hyphen-case, or longer than 64 characters) with `400`,
+  as delete and rollback already do, instead of `500` with a logged traceback on
+  every request. Only the name check maps to `400`: a corrupt history file
+  (malformed JSON or non-UTF-8) is still a `500`, and `POST .../rollback`, which
+  answered one with `400`, now returns `500` too. Deleting a skill with an
+  invalid name no longer logs a mutation-tail error. ([#7123])
+
 - **channels:** An IM message delivered twice no longer runs twice, and two
   Gateway instances no longer split a new conversation across two threads.
   Discord direct messages had no inbound dedupe key (no guild id), and a reply
@@ -11902,3 +11910,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
 [#7040]: https://github.com/bytedance/deer-flow/pull/7040
 [#7099]: https://github.com/bytedance/deer-flow/pull/7099
 [#7104]: https://github.com/bytedance/deer-flow/pull/7104
+[#7123]: https://github.com/bytedance/deer-flow/pull/7123

CHANGELOG_zh.md

@@ -1015,6 +1015,13 @@
 
 ### 修复
 
+- **技能：** `GET /api/skills/custom/{skill_name}/history` 收到非法技能名（不是
+  hyphen-case，或超过 64 个字符）时，现在与删除、回滚一样返回 `400`，不再返回 `500`
+  并在每次请求时记录一次带堆栈的错误日志。只有名称校验映射为 `400`：损坏的历史文件
+  （JSON 格式错误或非 UTF-8）仍返回 `500`，此前对其返回 `400` 的
+  `POST .../rollback` 现在也返回 `500`。用非法名称删除技能时不再记录变更尾部
+  （mutation tail）错误日志。 ([#7123])
+
 - **渠道：** 同一条 IM 消息被投递两次不再被执行两次，两个 Gateway 实例也不再把一个新
   会话拆到两个线程上。Discord 私信此前没有入站去重键（没有 guild id），而 Discord 线程
   内的回复在不跟踪该线程的实例上会得到另一个键；由于每个实例的会话都会收到每条消息，
@@ -9455,3 +9462,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子
 [#7040]: https://github.com/bytedance/deer-flow/pull/7040
 [#7099]: https://github.com/bytedance/deer-flow/pull/7099
 [#7104]: https://github.com/bytedance/deer-flow/pull/7104
+[#7123]: https://github.com/bytedance/deer-flow/pull/7123

backend/app/gateway/routers/skills.py

@@ -1,4 +1,5 @@
 import asyncio
+import json
 import logging
 import tempfile
 import threading
@@ -817,6 +818,9 @@ async def delete_custom_skill(skill_name: str, request: Request, config: AppConf
     await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
     try:
         skill_name = skill_name.replace("\r\n", "").replace("\n", "")
+        # Reject a malformed name (400 below) before the drained tail: failing
+        # there first takes the owner's mutation lock and logs a caller error.
+        SkillStorage.validate_skill_name(skill_name)
         # A cold user-scoped storage resolves the project root and builds
         # absolute paths on construction: blocking filesystem IO.
         storage = await asyncio.to_thread(_get_user_skill_storage, config)
@@ -863,6 +867,15 @@ async def get_custom_skill_history(skill_name: str, request: Request, config: Ap
     await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
     try:
         skill_name = skill_name.replace("\r\n", "").replace("\n", "")
+        # A malformed name is a client error (400, as delete and rollback
+        # answer). Validate it here rather than with a handler-wide
+        # ``except ValueError``: a corrupt history file raises
+        # ``json.JSONDecodeError`` or ``UnicodeDecodeError``, both ValueErrors,
+        # and must stay a 500.
+        try:
+            SkillStorage.validate_skill_name(skill_name)
+        except ValueError as e:
+            raise HTTPException(status_code=400, detail=str(e))
 
         def _read_history() -> list[dict] | None:
             # Worker thread: storage construction, the existence probes, and the
@@ -972,6 +985,12 @@ async def rollback_custom_skill(skill_name: str, body: SkillRollbackRequest, req
         raise HTTPException(status_code=400, detail="history_index is out of range")
     except FileNotFoundError as e:
         raise HTTPException(status_code=404, detail=str(e))
+    except (json.JSONDecodeError, UnicodeDecodeError) as e:
+        # A corrupt (malformed JSON or non-UTF-8) history file is a server
+        # fault, not a bad request; both are ValueErrors, so they must be
+        # caught before the 400 mapping below.
+        logger.error("Failed to roll back custom skill %s: %s", skill_name, e, exc_info=True)
+        raise HTTPException(status_code=500, detail=f"Failed to roll back custom skill: {str(e)}")
     except ValueError as e:
         raise HTTPException(status_code=400, detail=str(e))
     except Exception as e:

backend/docs/API.md

@@ -1189,6 +1189,9 @@ GET /api/skills/custom/{skill_name}/history
 }
 ```
 
+An invalid skill name returns `400`; a skill with neither a `SKILL.md` nor
+recorded history returns `404`.
+
 #### Rollback Custom Skill
 
 Restore a custom skill's `SKILL.md` from a history entry. The restored content is

backend/packages/harness/deerflow/skills/AGENTS.md

@@ -4,7 +4,7 @@
 
 - **Management YAML diagnostics**: `parse_skill_file(..., diagnostics=...)` optionally collects `SkillLoadDiagnostic` values separately from valid `Skill` entries. Only `UserScopedSkillStorage.load_custom_skill_diagnostics()` supplies that sink, after validating root ancestors and each resolved file against the caller's custom root. Exclude legacy/shared locations and external symlinks; do not weaken runtime discovery's separate compatibility policy. `GET /api/skills/diagnostics/custom` is admin-only and returns stable codes plus package-relative identifiers and 1-based file positions. Never return `_format_yaml_error`, exceptions or source excerpts. The management UI posts the existing admin-only reload before refetching diagnostics and normal skills; active Agent snapshots are unchanged.
 - **Diagnostic scope**: only `yaml.YAMLError` produces a management diagnostic. Missing frontmatter, non-mapping or invalid metadata, non-UTF-8 files and read errors (including permission denied) remain excluded; the UI and README state those limits. An empty diagnostic list is not a package-validity check. `tests/test_skill_load_diagnostics.py` pins these exclusions and permission-error redaction without changing runtime parsing.
-- **History JSONL**: split records on LF, not Unicode text separators; preserve U+0085/U+2028/U+2029 in saved fields. Keep malformed JSON rejected. Tests: `tests/test_skill_history_jsonl.py`.
+- **History JSONL**: split records on LF, not Unicode text separators; preserve U+0085/U+2028/U+2029 in saved fields. Keep malformed JSON rejected. Tests: `tests/test_skill_history_jsonl.py`. The history and rollback routes keep it a 500 and map only an invalid name to 400; `JSONDecodeError` and `UnicodeDecodeError` are `ValueError`s, so never let a handler-wide `except ValueError` catch them (`tests/test_skills_custom_router.py`).
 
 - **SkillScan usage counters**: the text assignment sweep recognizes unquoted nonnegative integers under the exact model-usage names `tokens`, `total_tokens`, `input_tokens`, `output_tokens`, `prompt_tokens`, `completion_tokens`, `cached_tokens`, and `reasoning_tokens`. Benchmark `tokens` also accepts a signed numeric delta or an object containing exactly four nonnegative finite numeric statistics (`mean`, `stddev`, `min`, `max`); JSON parsing is capped at 512 characters. Recover the complete key before applying the exception. Quoted unsigned counts, credential prefixes, arbitrary containers, and cloud-token detection remain active. Regressions live in `tests/test_skillscan_secret_assignment_formats.py`.
 

backend/tests/test_skills_custom_router.py

@@ -778,6 +778,87 @@ def test_custom_skill_delete_preserves_history_and_allows_restore(monkeypatch, t
         assert refresh_calls == [("refresh", "default"), ("refresh", "default")]
 
 
+def _make_demo_custom_skill_app(monkeypatch, tmp_path: Path) -> FastAPI:
+    from deerflow.config.paths import Paths
+
+    custom_dir = _user_custom_dir(tmp_path, "default") / "demo-skill"
+    custom_dir.mkdir(parents=True, exist_ok=True)
+    (custom_dir / "SKILL.md").write_text(_skill_content("demo-skill"), encoding="utf-8")
+
+    config = SimpleNamespace(
+        skills=SimpleNamespace(get_skills_path=lambda: tmp_path / "skills", container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage"),
+        skill_evolution=SimpleNamespace(enabled=True, moderation_model_name=None),
+    )
+    monkeypatch.setattr("deerflow.config.get_app_config", lambda: config)
+    monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: Paths(base_dir=tmp_path))
+    monkeypatch.setattr("deerflow.config.paths._paths", None)
+    monkeypatch.setattr("app.gateway.routers.skills.get_effective_user_id", lambda: "default")
+    return _make_test_app(config)
+
+
+@pytest.mark.parametrize(
+    ("skill_name", "detail"),
+    [
+        ("Bad_Name", "hyphen-case"),
+        ("a" * 65, "64 characters or fewer"),
+    ],
+)
+def test_custom_skill_history_rejects_invalid_name(monkeypatch, tmp_path, caplog, skill_name, detail):
+    """An invalid name is a 400 like delete and rollback, not a logged 500."""
+    import logging
+
+    app = _make_demo_custom_skill_app(monkeypatch, tmp_path)
+
+    with TestClient(app) as client, caplog.at_level(logging.ERROR, logger=skills_router.__name__):
+        response = client.get(f"/api/skills/custom/{skill_name}/history")
+
+    assert response.status_code == 400
+    assert detail in response.json()["detail"]
+    assert "Failed to read history" not in caplog.text
+
+
+def test_custom_skill_delete_rejects_invalid_name_before_mutation_tail(monkeypatch, tmp_path, caplog):
+    """A malformed name is a 400 the drained delete tail never sees, so nothing is logged."""
+    import logging
+
+    app = _make_demo_custom_skill_app(monkeypatch, tmp_path)
+
+    with TestClient(app) as client, caplog.at_level(logging.ERROR, logger=skills_router.__name__):
+        response = client.delete("/api/skills/custom/Bad_Name")
+
+    assert response.status_code == 400
+    assert "hyphen-case" in response.json()["detail"]
+    assert caplog.records == []
+
+
+@pytest.mark.parametrize(
+    "history_bytes",
+    [
+        pytest.param(b"{not json\n", id="malformed-json"),
+        pytest.param(b'{"action": "human_edit", "prev_content": "\xff"}\n', id="non-utf8"),
+    ],
+)
+@pytest.mark.parametrize(
+    ("method", "path", "body", "detail"),
+    [
+        ("GET", "/api/skills/custom/demo-skill/history", None, "Failed to read history:"),
+        ("POST", "/api/skills/custom/demo-skill/rollback", {"history_index": -1}, "Failed to roll back custom skill:"),
+    ],
+)
+def test_custom_skill_corrupt_history_record_stays_server_error(monkeypatch, tmp_path, method, path, body, detail, history_bytes):
+    """Only the name maps to 400: a corrupt history file (JSONDecodeError and UnicodeDecodeError are ValueErrors too) is a server fault."""
+    app = _make_demo_custom_skill_app(monkeypatch, tmp_path)
+    history_file = _user_custom_dir(tmp_path, "default") / ".history" / "demo-skill.jsonl"
+    history_file.parent.mkdir(parents=True, exist_ok=True)
+    history_file.write_bytes(history_bytes)
+
+    with TestClient(app) as client:
+        response = client.request(method, path, json=body)
+
+    assert response.status_code == 500
+    assert response.json()["detail"].startswith(detail)
+
+
 def test_custom_skill_delete_continues_when_history_write_is_readonly(monkeypatch, tmp_path):
     skills_root = tmp_path / "skills"
     from deerflow.config.paths import Paths