From be41ad2c0a145fa2ef9557db63a6c6afd8d22f4c Mon Sep 17 00:00:00 2001 From: host Date: Fri, 30 Oct 2026 18:26:55 +0000 Subject: [PATCH] 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. --- cloud/skill_seed.py | 21 +++ cloud/tests/test_skill_appendix_sync.py | 218 ++++++++++++++++++++++++ cloud/tests/test_skill_seed_pinning.py | 150 ++++++++++++++++ 3 files changed, 389 insertions(+) create mode 100644 cloud/tests/test_skill_appendix_sync.py create mode 100644 cloud/tests/test_skill_seed_pinning.py diff --git a/cloud/skill_seed.py b/cloud/skill_seed.py index 8022665..8ea51e2 100644 --- a/cloud/skill_seed.py +++ b/cloud/skill_seed.py @@ -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 diff --git a/cloud/tests/test_skill_appendix_sync.py b/cloud/tests/test_skill_appendix_sync.py new file mode 100644 index 0000000..7f9e0b9 --- /dev/null +++ b/cloud/tests/test_skill_appendix_sync.py @@ -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() diff --git a/cloud/tests/test_skill_seed_pinning.py b/cloud/tests/test_skill_seed_pinning.py new file mode 100644 index 0000000..e7f798d --- /dev/null +++ b/cloud/tests/test_skill_seed_pinning.py @@ -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()