-
Notifications
You must be signed in to change notification settings - Fork 11.2k
fix(skills): read skill markdown as UTF-8 #4995
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
6e9e48f
f488b81
e2bea01
5a5741f
a5c0d75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ def validate_skill(skill_path): | |
| return False, "SKILL.md not found" | ||
|
|
||
| # Read and validate frontmatter | ||
| content = skill_md.read_text() | ||
| content = skill_md.read_text(encoding="utf-8") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With a strict UTF-8 decode, a Suggest wrapping the read in |
||
| if not content.startswith('---'): | ||
| return False, "No YAML frontmatter found" | ||
|
|
||
|
|
@@ -99,4 +99,4 @@ def validate_skill(skill_path): | |
|
|
||
| valid, message = validate_skill(sys.argv[1]) | ||
| print(message) | ||
| sys.exit(0 if valid else 1) | ||
| sys.exit(0 if valid else 1) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import importlib.util | ||
| from pathlib import Path | ||
|
|
||
| REPO_ROOT = Path(__file__).resolve().parents[2] | ||
| VALIDATOR_PATH = REPO_ROOT / "skills" / "public" / "skill-creator" / "scripts" / "quick_validate.py" | ||
|
|
||
|
|
||
| def _load_validator(): | ||
| spec = importlib.util.spec_from_file_location("deerflow_skill_creator_quick_validate", VALIDATOR_PATH) | ||
| assert spec is not None | ||
| assert spec.loader is not None | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module | ||
|
|
||
|
|
||
| def test_validate_skill_reads_markdown_as_utf8(tmp_path: Path, monkeypatch) -> None: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This regression test is not wired into CI. No GitHub Actions workflow and no root Worth adding this directory to a workflow (or a root |
||
| validator = _load_validator() | ||
| skill_dir = tmp_path / "localized-skill" | ||
| skill_dir.mkdir() | ||
| skill_md = skill_dir / "SKILL.md" | ||
| skill_md.write_text( | ||
| "---\nname: localized-skill\ndescription: 处理中文内容\n---\n\n# 中文技能\n", | ||
| encoding="utf-8", | ||
| ) | ||
|
|
||
| original_read_text = Path.read_text | ||
|
|
||
| def require_explicit_encoding(self: Path, encoding: str | None = None, errors: str | None = None) -> str: | ||
| if self == skill_md and encoding is None: | ||
| raise UnicodeDecodeError("gbk", b"\x80", 0, 1, "illegal multibyte sequence") | ||
| return original_read_text(self, encoding=encoding, errors=errors) | ||
|
|
||
| monkeypatch.setattr(Path, "read_text", require_explicit_encoding) | ||
|
|
||
| assert validator.validate_skill(skill_dir) == (True, "Skill is valid!") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix is correct but incomplete against the contract this PR adds to AGENTS.md.
parse_skill_md()inskills/public/skill-creator/scripts/utils.py:9still does(skill_path / "SKILL.md").read_text()with the platform codec, and it is the SKILL.md reader used byrun_eval.py:279,run_loop.py:64andrun_loop.py:268, andimprove_description.py:213— so a localized UTF-8 skill still hits the exactUnicodeDecodeErrorthis PR fixes, just through a different script.Related, same root cause:
init_skill.py:230writesSKILL.mdviawrite_text(skill_content)with no encoding, so on a non-UTF-8 Windows code page skill-creator emits aSKILL.mdthat already violates the UTF-8 rule. The JSON reads inrun_eval.py:272,run_loop.py:261,improve_description.py:208/211andgenerate_report.py:314are locale-dependent too.