fix(skills): strip a leading BOM from the SKILL.md content handed to the model (#6984)

* fix(skills): strip a leading BOM from the SKILL.md content handed to the model

* fix(agents): hash the same BOM-free skill content the middleware injects

* docs(skills): state the BOM mechanism as it is, and sync the encoding convention

* test(skills): name the real BOM mechanism in the activation test docstring

* fix(ci): satisfy lint checks for skill BOM handling

---------

Co-authored-by: Jay0130-a <Jay0130-a@users.noreply.github.com>
Co-authored-by: Jay0130-a <309071492+Jay0130-a@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Jay0130-a 2026-10-18 22:03:27 +08:00 committed by GitHub
parent 816ebd5563
commit 62130caa82
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 79 additions and 6 deletions

View File

@ -214,9 +214,9 @@ Repo-wide conventions:
with the optional import unavailable and mock HTTP transport to keep tests offline.
- **Before pushing** — backend `make format`, frontend `pnpm check`.
Backend CI enforces `ruff format --check`.
- **Skill text encoding** — treat `SKILL.md` and other textual skill resources as UTF-8;
Python utilities that read or write them must pass `encoding="utf-8"` rather than
relying on the platform locale.
- **Skill text encoding** — use explicit UTF-8 for textual skill resources.
Read `SKILL.md` with `encoding="utf-8-sig"` to strip a leading BOM; write with
`encoding="utf-8"`. Never rely on the platform locale.
- **Version sources must stay in lockstep** — a release version must match identically in
`backend/pyproject.toml`, `frontend/package.json`, and `deploy/helm/deer-flow/Chart.yaml`
(`version` + `appVersion`), and `backend/uv.lock` must record the same version for the root

View File

@ -429,7 +429,10 @@ def _skill_content_hash(skill: object) -> str | None:
if not isinstance(skill_file, Path):
return None
try:
content = skill_file.read_text(encoding="utf-8")
# Same read as ``skill_activation_middleware._read_skill_content``, whose
# content this digest is defined to describe: ``utf-8-sig`` so a BOM a
# Windows editor wrote is not part of the digest.
content = skill_file.read_text(encoding="utf-8-sig")
except OSError:
return None
return canonical_hash(content)

View File

@ -304,7 +304,14 @@ class SkillActivationMiddleware(AgentMiddleware):
raise ValueError("Resolved skill file must stay within the configured skills root.") from exc
if not resolved_file.is_file():
raise FileNotFoundError(resolved_file)
return resolved_file.read_text(encoding="utf-8")
# ``utf-8-sig`` drops a leading UTF-8 BOM (Windows Notepad / PowerShell
# 5.1). The frontmatter path tolerates one already, but not by reading
# ``utf-8-sig``: ``_FRONTMATTER_RE`` matches an optional ``\ufeff`` before
# the opening fence and consumes it, so that tolerance covers the front
# matter only. This read returns the whole file, so plain ``utf-8`` would
# hand the model a body starting with U+FEFF and fingerprint that BOM in
# ``content_hash``.
return resolved_file.read_text(encoding="utf-8-sig")
def _resolve_activation(self, text: str, *, activation_decisions: ActivationDecisions | None = None) -> _ActivationResolution | None:
reference = parse_slash_skill_reference(text)

View File

@ -96,7 +96,11 @@ class LocalSkillStorage(SkillStorage):
with managed_read(self):
if not self.custom_skill_exists(name):
raise FileNotFoundError(f"Custom skill '{name}' not found.")
return (self.get_custom_skill_dir(name) / SKILL_MD_FILE).read_text(encoding="utf-8")
# Read BOM-tolerantly so a SKILL.md saved by a Windows editor does not
# return its BOM as the first character of the content this API hands
# back. The frontmatter path tolerates the BOM via ``_FRONTMATTER_RE``
# instead, which only covers the front matter, not this whole-file read.
return (self.get_custom_skill_dir(name) / SKILL_MD_FILE).read_text(encoding="utf-8-sig")
def write_custom_skill(self, name: str, relative_path: str, content: str) -> None:
with self._skill_projection_mutation(names=(name,)):

View File

@ -740,6 +740,28 @@ class TestSkillCatalogHashesContent:
assert before.fingerprint != after.fingerprint
assert before.enabled_skills == after.enabled_skills # the catalog's visible identity is unchanged
def test_a_bom_does_not_change_the_fingerprint(self, tmp_path):
"""A BOM is the editor's, not the skill's.
``_skill_content_hash`` digests the body ``SkillActivationMiddleware``
injects — its docstring says so and points at ``_read_skill_content`` —
and the middleware reads that file ``utf-8-sig``. The two must therefore
agree for a SKILL.md a Windows editor saved with a BOM, or the descriptor
would fingerprint content the agent never receives.
"""
skill_dir = tmp_path / "my-skill"
skill_dir.mkdir()
skill = self._skill(skill_dir)
body = "---\nname: my-skill\n---\nSame instructions.\n"
(skill_dir / "SKILL.md").write_text(body, encoding="utf-8")
without_bom = self._build([skill])
(skill_dir / "SKILL.md").write_text("\ufeff" + body, encoding="utf-8")
with_bom = self._build([skill])
assert with_bom.fingerprint == without_bom.fingerprint
def test_required_secrets_flag_changes_the_fingerprint(self):
from deerflow.skills.types import SecretRequirement

View File

@ -556,6 +556,43 @@ def test_skill_activation_middleware_reactivates_on_new_user_slash_command(monke
assert sum(1 for kwargs in recorded if kwargs.get("action") == "activate") == 2
def test_skill_activation_strips_a_leading_bom_from_skill_content(monkeypatch, tmp_path):
"""A SKILL.md saved by a Windows editor must reach the model without its BOM.
The frontmatter path already tolerates one — ``_FRONTMATTER_RE``
(``deerflow/skills/frontmatter.py``) consumes an optional leading BOM — so a
skill written by Notepad or PowerShell 5.1 parses and installs fine. That
tolerance covers the front matter only. Activation then reads the whole file
for the model, and reading it as plain ``utf-8`` there would hand the model a
body starting with U+FEFF and fingerprint that BOM in the ``sha256`` the
reminder publishes. This pins both halves.
"""
body = "# Data Analysis\nUse pandas."
skill = _make_skill(tmp_path, "data-analysis", content=body)
# Re-write the same content with a UTF-8 BOM, the way those editors save.
skill.skill_file.write_text("\ufeff" + body, encoding="utf-8")
monkeypatch.setattr(middleware_module, "get_or_new_skill_storage", lambda **kwargs: _make_storage(tmp_path, [skill]))
middleware = SkillActivationMiddleware(slash_source_owner_token=_SLASH_SOURCE_OWNER_TOKEN)
original = HumanMessage(content="/data-analysis analyze uploads/foo.csv", id="msg-1")
captured = {}
def handler(model_request: ModelRequest):
captured["messages"] = model_request.messages
return AIMessage(content="ok")
middleware.wrap_model_call(_make_model_request([original], runtime=SimpleNamespace(context=None)), handler)
reminders = [message for message in captured["messages"] if is_slash_skill_activation_reminder(message)]
assert len(reminders) == 1
content = reminders[0].content
assert "\ufeff" not in content
# The published digest fingerprints the skill, not the editor's byte-order mark:
# it equals the digest of the same file written without one.
assert f'sha256="{hashlib.sha256(body.encode("utf-8")).hexdigest()}"' in content
def test_skill_activation_middleware_activates_per_call_when_run_context_is_none(monkeypatch, tmp_path):
# Regression for the degraded-path contract: when runtime.context is None
# (e.g. no run-scoped context is threaded through - the #3989 case), _run_context()