test: pin skill appendix sync and SKILL.md-only seeding (C-line, no behaviour change)
- test_skill_appendix_sync: all 9 appendix chapters 00..08 in order, body role-matrix set == appendix role-title set, authority statement on both sides, GitHub guarantee in body + named appendix chapters 00/04/07, seeded copy byte-identical with full appendix. Previously only 00/08 existence was asserted. - test_skill_seed_pinning: only SKILL.md copied (pack extras never land, never resurrected), tenant extras left alone, tenant-modified SKILL.md never overwritten across repeats, identical = noop, deleted SKILL.md re-seeded (documented boundary, not deletion preservation). - skill_seed.py: evaluation note only (why whole-dir copy is not done); no code change.
This commit is contained in:
host committed
1 parent
06710cf47b
commit
be41ad2c0a
3 files changed
+389
No files matched your search
@@ -55,6 +55,27 @@ copying is not adopted unless a future evaluation proves both
|
||||
content-preservation AND deletion-preservation. Until then, only
|
||||
SKILL.md is seeded; extra files in a pack directory are ignored
|
||||
(pinned by test_merge_gate_protectskill.py).
|
||||
|
||||
Evaluation note (2026-10-31, skill-engineering convergence line;
|
||||
comment only, behaviour unchanged -- see
|
||||
cloud/tests/test_skill_seed_pinning.py, which pins it):
|
||||
whole-directory copy is explicitly NOT done, for three reasons.
|
||||
(1) The pack today ships exactly one file per skill (SKILL.md;
|
||||
verified: cloud/skills/*/ contains nothing else), so copying a
|
||||
whole directory has zero benefit today and would silently start
|
||||
copying future extras (templates/, README) without review.
|
||||
(2) Per-file skip-if-differ cannot preserve a tenant deletion:
|
||||
a missing destination is copied by design, so whole-dir copy
|
||||
would resurrect tenant-deleted auxiliary files on the next run.
|
||||
SKILL.md-only copy bounds that resurrection surface to the single
|
||||
SKILL.md file itself (auxiliary deletions are preserved trivially
|
||||
because those files never landed in the tenant home).
|
||||
(3) Seeding never prunes the tenant directory, so tenant-added
|
||||
files in a skill directory are left alone by the same rule.
|
||||
Boundary, stated plainly so it is not misread: a tenant-deleted
|
||||
SKILL.md IS re-seeded (missing = copied); only the differ-guard
|
||||
preserves edits. Preserving a SKILL.md deletion would need a
|
||||
tombstone, which is new behaviour, not a convergence fix.
|
||||
"""
|
||||
|
||||
import shutil
|
||||
|
||||
@@ -0,0 +1,218 @@
|
||||
"""Body<->appendix sync for the software-engineering skill (C-line).
|
||||
|
||||
Scope guard: this file READS cloud/skills/software-engineering/SKILL.md
|
||||
and asserts structure; it never edits the skill text. If any assertion
|
||||
here fails, the fix is a skill-content decision (edit SKILL.md through
|
||||
its own track), not a silent edit by this test.
|
||||
|
||||
Audit finding on origin/main (2026-10-31): there was NO bidirectional
|
||||
test at all. The only appendix coverage was test_skill_seed_worker.py
|
||||
asserting two chapters (00-coordinator, 08-shared-core) exist in the
|
||||
seeded file -- a dropped middle chapter (01..07) would pass CI silently.
|
||||
The bidirectional assertion previously discussed on the #209 track is
|
||||
unmerged, so main does not have it; this file supplies it on the main
|
||||
base (skill text v0.2, 9 appendix chapters 00..08).
|
||||
|
||||
What "bidirectional" means here, concretely (all derived from SKILL.md
|
||||
itself, no new semantics invented):
|
||||
- body names the appendix as the authority AND the appendix intro
|
||||
claims itself as the authority (either side missing -> fail naming
|
||||
the missing side);
|
||||
- body names exactly three appendix chapters for the GitHub guarantee
|
||||
(coordinator, client, acceptance) AND each named chapter carries
|
||||
the rule keywords (each chapter's keyword set is tailored to what
|
||||
that chapter actually says -- e.g. 07-acceptance carries force-push /
|
||||
merge-gate / PR-number rules but no literal "GitHub" word, so the
|
||||
test must not demand that word there);
|
||||
- body role-matrix role names == appendix role-title names as sets
|
||||
(order differs: matrix order vs numeric 00..08 order, so set, not
|
||||
list, comparison), and appendix ids are exactly 00..08 in order.
|
||||
|
||||
CI: .github/workflows/host-verify.yml and release.yml both run
|
||||
`python3 -m unittest discover -s cloud/tests -v`, so this test_*.py
|
||||
file runs in CI automatically with no workflow edit.
|
||||
|
||||
Only software-engineering has an appendix (grep: the other four
|
||||
skills contain zero "附录"); appendix checks are therefore scoped to
|
||||
this one skill and must NOT be generalised to the other skills.
|
||||
"""
|
||||
|
||||
import re
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
try:
|
||||
from cloud.skill_seed import ensure_codex_home_skills
|
||||
except ImportError:
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[2]))
|
||||
from cloud.skill_seed import ensure_codex_home_skills
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
SKILL_PATH = (REPO_ROOT / "cloud" / "skills" / "software-engineering"
|
||||
/ "SKILL.md")
|
||||
|
||||
EXPECTED_IDS = ["00-coordinator", "01-product-manager", "02-architect",
|
||||
"03-designer", "04-client-developer", "05-backend",
|
||||
"06-qa", "07-acceptance", "08-shared-core"]
|
||||
|
||||
# Chapter -> keywords that chapter must carry for the GitHub guarantee
|
||||
# the body claims is "同样写进了附录的协调员、客户端、验收章节".
|
||||
# 07 has no literal "GitHub" word (its rules live under
|
||||
# "按端发布与回滚（验收）"); demanding it would be a false failure.
|
||||
GITHUB_CHAPTER_KEYWORDS = {
|
||||
"00-coordinator": ["GitHub 保证", "force-push", "合并门"],
|
||||
"04-client-developer": ["GitHub 交付", "force-push", "PR 号"],
|
||||
"07-acceptance": ["force-push", "合并门", "PR 号"],
|
||||
}
|
||||
|
||||
CHAPTER_RE = re.compile(
|
||||
r"^## 附录角色：(\d{2}-[a-z0-9-]+)"
|
||||
r"（来源 templates/roles/(\d{2}-[a-z0-9-]+\.md)）",
|
||||
flags=re.M)
|
||||
TITLE_RE = re.compile(r"^# 角色模板：(.+)$", flags=re.M)
|
||||
|
||||
|
||||
def _read_skill():
|
||||
assert SKILL_PATH.is_file(), (
|
||||
"正文侧缺失：技能源文件不存在：%s" % SKILL_PATH)
|
||||
return SKILL_PATH.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _split(text):
|
||||
marker = "## 附录：角色模板"
|
||||
assert marker in text, "正文侧缺失：找不到附录引言节 %r" % marker
|
||||
body, appendix = text.split(marker, 1)
|
||||
return body, appendix
|
||||
|
||||
|
||||
def _chapters(appendix):
|
||||
matches = list(CHAPTER_RE.finditer(appendix))
|
||||
chapters = {}
|
||||
for i, m in enumerate(matches):
|
||||
end = matches[i + 1].start() if i + 1 < len(matches) else len(appendix)
|
||||
chapters[m.group(1)] = appendix[m.start():end]
|
||||
return chapters, matches
|
||||
|
||||
|
||||
def _base_role_name(title):
|
||||
# "后端 / API（可选，1 个线程）" -> "后端 / API";
|
||||
# "后端 / API（如有）" (matrix cell) -> "后端 / API".
|
||||
return re.sub(r"（[^）]*）\s*$", "", title.strip())
|
||||
|
||||
|
||||
class AppendixSyncTestCase(unittest.TestCase):
|
||||
def test_appendix_has_all_nine_chapters_in_order(self):
|
||||
text = _read_skill()
|
||||
_body, appendix = _split(text)
|
||||
chapters, matches = _chapters(appendix)
|
||||
found = [m.group(1) for m in matches]
|
||||
self.assertEqual(
|
||||
found, EXPECTED_IDS,
|
||||
"附录侧缺失/错序：附录章节 id 应为 %s，实际为 %s"
|
||||
% (EXPECTED_IDS, found))
|
||||
for cid in EXPECTED_IDS:
|
||||
sec = chapters[cid]
|
||||
src = [m.group(2) for m in matches if m.group(1) == cid][0]
|
||||
self.assertEqual(
|
||||
src, cid + ".md",
|
||||
"附录侧不一致：章节 %s 的来源注应为 %s，实际为 %s"
|
||||
% (cid, cid + ".md", src))
|
||||
title = TITLE_RE.search(sec)
|
||||
self.assertIsNotNone(
|
||||
title,
|
||||
"附录侧缺失：章节 %s 内找不到“# 角色模板：”标题行" % cid)
|
||||
# Smallest real chapter is ~600 chars; 300 catches a
|
||||
# dropped/emptied chapter without being brittle.
|
||||
self.assertGreater(
|
||||
len(sec), 300,
|
||||
"附录侧缺失/被掏空：章节 %s 仅 %d 字" % (cid, len(sec)))
|
||||
|
||||
def test_body_role_matrix_matches_appendix_titles(self):
|
||||
text = _read_skill()
|
||||
body, appendix = _split(text)
|
||||
chapters, _m = _chapters(appendix)
|
||||
appendix_roles = set()
|
||||
for cid, sec in chapters.items():
|
||||
title = TITLE_RE.search(sec)
|
||||
self.assertIsNotNone(title, "附录侧缺失：%s 无标题" % cid)
|
||||
appendix_roles.add(_base_role_name(title.group(1)))
|
||||
matrix_roles = set()
|
||||
# Only the role-matrix section: other body tables (gate table
|
||||
# with G0..G6, etc.) must not leak into the role set.
|
||||
matrix_section = body.split("## 角色线程矩阵", 1)[1]
|
||||
matrix_section = matrix_section.split("\n## ", 1)[0]
|
||||
for line in matrix_section.splitlines():
|
||||
if not line.startswith("| "):
|
||||
continue
|
||||
cells = [c.strip() for c in line.strip().strip("|").split("|")]
|
||||
if not cells or cells[0] in ("角色", "------"):
|
||||
continue
|
||||
matrix_roles.add(_base_role_name(cells[0]))
|
||||
self.assertEqual(
|
||||
matrix_roles, appendix_roles,
|
||||
"双向不一致：正文角色矩阵缺 %s；附录缺角色 %s"
|
||||
% (sorted(appendix_roles - matrix_roles),
|
||||
sorted(matrix_roles - appendix_roles)))
|
||||
|
||||
def test_authority_statement_is_bidirectional(self):
|
||||
text = _read_skill()
|
||||
body, appendix = _split(text)
|
||||
for phrase in ("角色模板的权威来源是本文件末尾的附录",
|
||||
"不一致时以附录为准"):
|
||||
self.assertIn(
|
||||
phrase, body,
|
||||
"正文侧缺失：正文未声明“%s”（附录单方面有权威也无法核对）"
|
||||
% phrase)
|
||||
intro = appendix.split("## 附录角色：", 1)[0]
|
||||
for phrase in ("以本附录为准", "核对附录与 `templates/roles/` 一致"):
|
||||
self.assertIn(
|
||||
phrase, intro,
|
||||
"附录侧缺失：附录引言未声明“%s”（正文单方面指认附录也无法核对）"
|
||||
% phrase)
|
||||
|
||||
def test_github_guarantee_in_body_and_named_appendix_chapters(self):
|
||||
text = _read_skill()
|
||||
body, appendix = _split(text)
|
||||
chapters, _m = _chapters(appendix)
|
||||
for phrase in ("同样写进了附录的协调员、客户端、验收章节",
|
||||
"正文与附录不一致就是不合格"):
|
||||
self.assertIn(
|
||||
phrase, body,
|
||||
"正文侧缺失：正文 GitHub 保证节未声明“%s”" % phrase)
|
||||
for cid, keywords in GITHUB_CHAPTER_KEYWORDS.items():
|
||||
self.assertIn(
|
||||
cid, chapters, "附录侧缺失：正文点名的章节 %s 不在附录中" % cid)
|
||||
for kw in keywords:
|
||||
self.assertIn(
|
||||
kw, chapters[cid],
|
||||
"附录侧缺失：章节 %s 缺少规则关键词 %r"
|
||||
"（正文 GitHub 保证节声称该规则已写进本章）"
|
||||
% (cid, kw))
|
||||
|
||||
def test_seeded_copy_carries_full_appendix_byte_identical(self):
|
||||
# Delivery-channel check: the seeded tenant copy must carry
|
||||
# the whole appendix, not just the two chapters the old test
|
||||
# checked. Byte-identity is the strongest form: skill_seed
|
||||
# copies SKILL.md verbatim, so any appendix edit in the pack
|
||||
# reaches the tenant only through this exact file.
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
out = ensure_codex_home_skills(tmp)
|
||||
self.assertIn("software-engineering", out["skills_copied"])
|
||||
seeded = Path(tmp) / "skills" / "software-engineering" \
|
||||
/ "SKILL.md"
|
||||
self.assertTrue(seeded.is_file(), "播种侧缺失：未生成租户 SKILL.md")
|
||||
seeded_text = seeded.read_text(encoding="utf-8")
|
||||
self.assertEqual(
|
||||
seeded_text, _read_skill(),
|
||||
"播种侧不一致：租户 SKILL.md 与源包字节不一致（附录可能被截断）")
|
||||
_body, appendix = _split(seeded_text)
|
||||
chapters, _m = _chapters(appendix)
|
||||
self.assertEqual(
|
||||
sorted(chapters), sorted(EXPECTED_IDS),
|
||||
"播种侧缺失：租户副本附录章节不全：%s" % sorted(chapters))
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -0,0 +1,150 @@
|
||||
"""Pin skill_seed behaviour (C-line): SKILL.md-only, never overwrite.
|
||||
|
||||
These are helper-level tests (no DB, no Worker) for
|
||||
cloud.skill_seed.ensure_codex_home_skills, complementing the
|
||||
worker-level tests in test_skill_seed_worker.py (which already cover
|
||||
flag-off / flag-on / tenant-edit-survives through _tenant_dirs).
|
||||
|
||||
Two behaviours are pinned here, behaviour itself unchanged:
|
||||
|
||||
1. Only SKILL.md is ever copied. A skill directory may contain extra
|
||||
files (README, templates/, notes); none of them may land in the
|
||||
tenant home, and re-seeding must never resurrect them. Rationale
|
||||
(see the evaluation note in skill_seed.py): the pack today ships
|
||||
only SKILL.md per skill, so whole-directory copy has zero benefit
|
||||
today and would silently start copying future extras without
|
||||
review; per-file skip-if-differ cannot preserve a tenant deletion
|
||||
(missing destination is copied), so whole-dir copy would
|
||||
resurrect tenant-deleted auxiliary files on the next run.
|
||||
SKILL.md-only copy bounds that resurrection surface to the single
|
||||
SKILL.md file itself.
|
||||
2. A tenant-modified SKILL.md is never overwritten. It is reported
|
||||
in skills_skipped_differ and left byte-identical, across repeats,
|
||||
while other skills still seed normally.
|
||||
|
||||
Documented boundary, asserted explicitly rather than hidden: a
|
||||
tenant-DELETED SKILL.md is re-seeded on the next run (missing =
|
||||
copied). That is the existing, intended meaning of "missing is
|
||||
copied"; preserving a deletion would need a tombstone, which is new
|
||||
behaviour and out of scope for this convergence line. The test names
|
||||
it so a future reader does not mistake re-seeding for a bug -- or
|
||||
mistake the differ-guard for deletion preservation.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
try:
|
||||
from cloud.skill_seed import ensure_codex_home_skills, list_skills
|
||||
except ImportError:
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[2]))
|
||||
from cloud.skill_seed import ensure_codex_home_skills, list_skills
|
||||
|
||||
|
||||
def _pack(root):
|
||||
"""Pack with extras that must never reach the tenant home."""
|
||||
pack = Path(root) / "pack"
|
||||
skill_a = pack / "skill-a"
|
||||
(skill_a / "templates" / "roles").mkdir(parents=True, exist_ok=True)
|
||||
(skill_a / "SKILL.md").write_text("---\nname: skill-a\n---\nbody-a\n")
|
||||
(skill_a / "README.md").write_text("pack readme, must not seed\n")
|
||||
(skill_a / "notes.txt").write_text("pack notes, must not seed\n")
|
||||
(skill_a / "templates" / "roles" / "00-x.md").write_text("role\n")
|
||||
skill_b = pack / "skill-b"
|
||||
skill_b.mkdir(parents=True, exist_ok=True)
|
||||
(skill_b / "SKILL.md").write_text("---\nname: skill-b\n---\nbody-b\n")
|
||||
# Directory without SKILL.md: reported, never invented.
|
||||
(pack / "skill-broken").mkdir(parents=True, exist_ok=True)
|
||||
(pack / "skill-broken" / "other.txt").write_text("no SKILL.md here\n")
|
||||
return pack
|
||||
|
||||
|
||||
def _dest_tree(home):
|
||||
out = []
|
||||
root = Path(home) / "skills"
|
||||
if root.is_dir():
|
||||
for p in sorted(root.rglob("*")):
|
||||
if p.is_file():
|
||||
out.append(str(p.relative_to(root)))
|
||||
return out
|
||||
|
||||
|
||||
class SkillSeedPinningTestCase(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.TemporaryDirectory()
|
||||
|
||||
def tearDown(self):
|
||||
self.tmp.cleanup()
|
||||
|
||||
def test_only_skill_md_is_copied_never_extras(self):
|
||||
home = os.path.join(self.tmp.name, "home")
|
||||
out = ensure_codex_home_skills(home, _pack(self.tmp.name))
|
||||
self.assertEqual(sorted(out["skills_copied"]),
|
||||
["skill-a", "skill-b"])
|
||||
self.assertEqual(out["skills_skipped_missing"], ["skill-broken"])
|
||||
self.assertEqual(
|
||||
_dest_tree(home),
|
||||
["skill-a/SKILL.md", "skill-b/SKILL.md"])
|
||||
# Second run: still no extras, and no resurrection of anything
|
||||
# the tenant never had -- copied is empty because both SKILL.md
|
||||
# files are now identical.
|
||||
out2 = ensure_codex_home_skills(home, _pack(self.tmp.name))
|
||||
self.assertEqual(out2["skills_copied"], [])
|
||||
self.assertEqual(
|
||||
_dest_tree(home),
|
||||
["skill-a/SKILL.md", "skill-b/SKILL.md"])
|
||||
|
||||
def test_tenant_extra_files_in_dest_are_left_alone(self):
|
||||
home = os.path.join(self.tmp.name, "home")
|
||||
ensure_codex_home_skills(home, _pack(self.tmp.name))
|
||||
tenant_note = Path(home) / "skills" / "skill-a" / "tenant-notes.md"
|
||||
tenant_note.write_text("tenant's own file\n")
|
||||
ensure_codex_home_skills(home, _pack(self.tmp.name))
|
||||
self.assertEqual(tenant_note.read_text(), "tenant's own file\n")
|
||||
self.assertIn("skill-a/tenant-notes.md", _dest_tree(home))
|
||||
# Seeding never prunes the tenant directory either.
|
||||
self.assertIn("skill-a/SKILL.md", _dest_tree(home))
|
||||
|
||||
def test_tenant_modified_skill_never_overwritten_across_repeats(self):
|
||||
home = os.path.join(self.tmp.name, "home")
|
||||
pack = _pack(self.tmp.name)
|
||||
ensure_codex_home_skills(home, pack)
|
||||
dest = Path(home) / "skills" / "skill-a" / "SKILL.md"
|
||||
dest.write_text("tenant edited version\n")
|
||||
for _ in range(2):
|
||||
out = ensure_codex_home_skills(home, pack)
|
||||
self.assertEqual(out["skills_skipped_differ"], ["skill-a"])
|
||||
self.assertEqual(dest.read_text(), "tenant edited version\n")
|
||||
# The other skill is unaffected by one tenant edit.
|
||||
dest_b = Path(home) / "skills" / "skill-b" / "SKILL.md"
|
||||
self.assertEqual(dest_b.read_text(),
|
||||
"---\nname: skill-b\n---\nbody-b\n")
|
||||
|
||||
def test_identical_dest_is_noop_and_missing_is_recopied(self):
|
||||
home = os.path.join(self.tmp.name, "home")
|
||||
pack = _pack(self.tmp.name)
|
||||
ensure_codex_home_skills(home, pack)
|
||||
out = ensure_codex_home_skills(home, pack)
|
||||
self.assertEqual(out["skills_copied"], [])
|
||||
self.assertEqual(out["skills_skipped_differ"], [])
|
||||
# Documented boundary (see module docstring): a tenant-deleted
|
||||
# SKILL.md is re-seeded -- missing means copied. This is NOT
|
||||
# deletion preservation; only the differ-guard preserves edits.
|
||||
dest_b = Path(home) / "skills" / "skill-b" / "SKILL.md"
|
||||
dest_b.unlink()
|
||||
out2 = ensure_codex_home_skills(home, pack)
|
||||
self.assertEqual(out2["skills_copied"], ["skill-b"])
|
||||
self.assertTrue(dest_b.is_file())
|
||||
|
||||
def test_list_skills_ignores_broken_and_non_directories(self):
|
||||
pack = _pack(self.tmp.name)
|
||||
(pack / "stray.txt").write_text("not a skill\n")
|
||||
names = [n for n, _p in list_skills(pack)]
|
||||
self.assertEqual(names, ["skill-a", "skill-b"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user