From eaa7dac4a416cdf305dc101cca885026d408097a Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Fri, 30 Oct 2026 21:56:13 -0700 Subject: [PATCH 01/22] feat(catalog): check bundle links, descriptions and mirror drift `catalog scan` reviews each loaded skill beyond the spec frontmatter: - a relative Markdown link to a file the skill does not have is a warn (link-missing), with a note when the file exists from the catalog root instead. Example links such as [Title](URL) are common, so this does not fail --strict. A link or ../ code span that reaches outside the skill directory is a warn (link-escapes-skill, path-escapes-skill). Code fences, URLs, anchors and code spans naming no file are not checked. - a description with no "when to use" clause is a warn (description-no-when); one whose words overlap another skill's at a Jaccard similarity of at least 0.5 is a warn on both (description-near-duplicate). - two skills in different paths with the same directory name and different SKILL.md text are a warn on both (mirror-drift), naming the other copies. Skills with disable-model-invocation: true are exempt from the description checks. Warnings never change the exit code. The review reads the scan's own entries, which keep byte-identical copies (discover_skills counts a mirror once), so a mirror is still seen here. Signed-off-by: Nikolay Petrov --- ...00_napetrov_catalog-bundle-drift-checks.md | 14 ++ dokimi/docs/external-content.md | 13 ++ dokimi/dokimi/external/validate.py | 207 ++++++++++++++++++ dokimi/tests/test_catalog_validate.py | 203 ++++++++++++++++- 4 files changed, 434 insertions(+), 3 deletions(-) create mode 100644 dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md diff --git a/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md b/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md new file mode 100644 index 000000000..b078b8b9b --- /dev/null +++ b/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md @@ -0,0 +1,14 @@ +### Added + +- **`catalog scan` checks each skill's bundle and description, and detects drifted mirrors** — + a relative Markdown link in `SKILL.md` to a file that does not exist is a `warn` + (`link-missing`), and says so when the file exists from the catalog root instead (a wrong + base). A link or `../` code span that reaches a file outside the skill directory is a `warn` + (`link-escapes-skill`, `path-escapes-skill`): it breaks when the skill is installed alone. + A description with no "when to use" clause is a `warn` (`description-no-when`), as is one + whose words overlap another skill's by at least half (`description-near-duplicate`). Two + skills in different paths with the same directory name and different `SKILL.md` text are + a `warn` on both (`mirror-drift`), naming the other copies. Links inside code fences, URLs, + anchors, and code spans that name no file are not checked. Skills with + `disable-model-invocation: true` are exempt from the description checks. Warnings never + change the exit code; only `--strict` makes an `error` fail. diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index 4eb88f845..4cc80d246 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -129,6 +129,19 @@ offers; each other copy is the `warn` and names it. Warnings never change the exit code. `disable-model-invocation: true` is printed as a `note`, because the model is never offered that skill. +The scan also checks the bundle and the description. A relative Markdown link +to a file the skill does not have is a `warn` (`link-missing`), and says when the +file exists from the catalog root instead. Example links such as `[Title](URL)` +are common, so `--strict` does not fail on them. A link, or a `../` code span, +that reaches a file outside the skill directory is a `warn` +(`link-escapes-skill`, `path-escapes-skill`), since it breaks when the skill is +installed alone. A description with no "when to use" clause is a `warn` +(`description-no-when`), and so is one that overlaps another skill's by at least +half (`description-near-duplicate`). Two skills in different paths with the same +directory name and different `SKILL.md` text are a `warn` on both +(`mirror-drift`), naming the other copies. Skills with +`disable-model-invocation: true` are exempt from the description checks. + Frontmatter that is not valid YAML (a bare colon in `description:`, say) stops the skill from loading, since the loader reads it as having no description. The scan reports that as a `frontmatter-parse` error carrying the YAML message, and diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index ff13c2c71..3b1c1f518 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -30,6 +30,23 @@ ``disable-model-invocation: true`` is not a finding: the model is never offered that skill, so it is reported as a flag (the same reading as the skill arm's check in :mod:`dokimi.harnesses.skill_treatment`). + +Bundle links: a relative Markdown link in ``SKILL.md`` whose file does not exist is a +``warn`` (``link-missing``), with a note when the file exists from the catalog root +instead (a wrong base). Example links such as ``[Title](URL)`` are common, so this is not +an error and does not fail ``--strict``. A link to a file outside the skill directory is a ``warn`` +(``link-escapes-skill``): it breaks when the skill is installed alone. A ``../`` code span +naming such a file is ``path-escapes-skill``. Code spans that name no file are not checked, +and neither are code fences: those are usually examples or paths in the user's own project. + +Descriptions: one with no "when to use" clause is a ``warn`` (``description-no-when``), +and so is one whose tokens overlap another skill's at a Jaccard similarity of at least +:data:`NEAR_DUPLICATE_JACCARD` (``description-near-duplicate``). Skills with +``disable-model-invocation: true`` are exempt from both, as the model never chooses them. + +Mirror drift: two skills in different paths with the same directory name but different +``SKILL.md`` text are a ``warn`` on both (``mirror-drift``), naming the other copies: the +same skill has probably been edited in one place and not the other. """ from __future__ import annotations @@ -50,6 +67,28 @@ NAME_PATTERN = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*") NAME_MAX = 64 DESCRIPTION_MAX = 1024 +NEAR_DUPLICATE_JACCARD = 0.5 + +# A "when to use" clause: the model chooses a skill from its description alone. +WHEN_RE = re.compile( + r"\b(?:when to use|when to delegate|use when|use this when|use for|use this skill|" + r"trigger(?:s|ed)?\s*(?:on|when|:)|invoke (?:when|this)|invoke only|" + r"use (?:it |this |these )?(?:when|if|before|after|whenever|proactively|anytime)|" + r"whenever|when the user|activate\b|should be used when|when building|" + r"when (?:you|the user) )", + re.I, +) +# Words that say nothing about what a skill is for; left out of the description overlap. +STOP_WORDS = frozenset( + """a an the and or of to in on for with by from as at is are be been being this that + these those it its it's you your we our use used using when with without into than then + also any all each skill skills claude user users make makes do does not no yes can will + should must may might if else only other such via per about over under more most very + just""".split() +) +LINK_RE = re.compile(r"\]\(([^)\s]+)(?:\s+\"[^\"]*\")?\)") +CODE_SPAN_RE = re.compile(r"`([^`\n]+)`") +SKILL_DIR_VAR = "${CLAUDE_SKILL_DIR}/" @dataclass @@ -184,15 +223,177 @@ def _relative_to_root(path: Path, root: Path) -> str: return path.as_posix() +def _outside_fences(body: str) -> list[str]: + """The lines of ``body`` that are not inside a fenced code block.""" + lines: list[str] = [] + fenced = False + for line in body.splitlines(): + if line.lstrip().startswith(("```", "~~~")): + fenced = not fenced + elif not fenced: + lines.append(line) + return lines + + +def _file_ref(raw: str) -> Optional[str]: + """The skill-relative file a reference names, or None if it names no file to check. + + None for a URL, a bare anchor, an absolute path, or a template with a variable other + than ``${CLAUDE_SKILL_DIR}``. + """ + text = raw.split("#", 1)[0].strip() + if not text or "://" in text or text.startswith(("mailto:", "/")): + return None + if text.startswith(SKILL_DIR_VAR): + return text[len(SKILL_DIR_VAR) :] or None + if any(c in text for c in "${}<>*"): + return None + return text + + +def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: + """Link and path checks on the body of ``skill``: missing targets, or targets outside it.""" + skill_dir = skill.path.parent + real_dir = skill_dir.resolve() + lines = _outside_fences(skill.body) + links = dict.fromkeys(m for line in lines for m in LINK_RE.findall(line)) + spans = dict.fromkeys( + s.strip() + for line in lines + for s in CODE_SPAN_RE.findall(line) + if s.strip().startswith("../") + ) + findings: list[Finding] = [] + for raw in links: + text = _file_ref(raw) + if text is None: + continue + target = (skill_dir / text).resolve() + if target.exists(): + if not target.is_relative_to(real_dir): + findings.append( + Finding( + "warn", + "link-escapes-skill", + f"link {raw!r} points outside the skill directory; it breaks when " + "the skill is installed alone", + ) + ) + continue + from_root = "" + if (source_root / text).exists(): + from_root = " (it exists from the catalog root: the base is wrong)" + findings.append( + Finding( + "warn", + "link-missing", + f"link {raw!r} names no file relative to the skill directory{from_root}", + ) + ) + for raw in spans: + text = _file_ref(raw) + if text is None: + continue + target = (skill_dir / text).resolve() + if target.exists() and not target.is_relative_to(real_dir): + findings.append( + Finding( + "warn", + "path-escapes-skill", + f"{raw!r} points outside the skill directory; it breaks when the skill " + "is installed alone", + ) + ) + return findings + + +def _description_findings(skill: Skill) -> list[Finding]: + """A ``warn`` when the description has no "when to use" clause.""" + if not WHEN_RE.search(" ".join(skill.description.split())): + return [ + Finding( + "warn", + "description-no-when", + "description has no 'when to use' or 'use when' clause; the model chooses " + "the skill from the description alone", + ) + ] + return [] + + +def _tokens(description: str) -> frozenset[str]: + """The lowercase words of three or more letters in ``description``, less the stop words.""" + words = re.findall(r"[a-z]{3,}", description.lower()) + return frozenset(w for w in words if w not in STOP_WORDS) + + +def _jaccard(a: frozenset[str], b: frozenset[str]) -> float: + return len(a & b) / len(a | b) if a and b else 0.0 + + +def _mirror_drift(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) -> None: + """``mirror-drift`` on each skill that shares its directory name with a differing copy.""" + by_dir_name: dict[str, list[tuple[SkillCatalogEntry, Skill, SkillReview]]] = {} + for item in loaded: + by_dir_name.setdefault(item[1].path.parent.name, []).append(item) + for group in by_dir_name.values(): + for _, _, review in group: + others = [ + _relative_to_root(o_skill.path.parent, o_entry.source_root) + for o_entry, o_skill, o_review in group + if o_review.content_sha256 != review.content_sha256 + ] + if others: + review.findings.append( + Finding( + "warn", + "mirror-drift", + f"SKILL.md differs from {', '.join(others)} (the same skill name in " + "another path); the copies may have drifted apart", + ) + ) + + +def _near_duplicates(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) -> None: + """``description-near-duplicate`` on both skills of each pair whose descriptions overlap.""" + shown = [item for item in loaded if not item[2].disable_model_invocation] + tokens = [_tokens(" ".join(item[1].description.split())) for item in shown] + for i, (entry, skill, review) in enumerate(shown): + for j in range(i + 1, len(shown)): + o_entry, o_skill, o_review = shown[j] + if o_review.content_sha256 == review.content_sha256: + continue # an identical SKILL.md: already a duplicate + if o_skill.path.parent.name == skill.path.parent.name: + continue # the same skill in another path: already a mirror-drift + similarity = _jaccard(tokens[i], tokens[j]) + if similarity >= NEAR_DUPLICATE_JACCARD: + pairs = ( + (review, _relative_to_root(o_skill.path.parent, o_entry.source_root)), + (o_review, _relative_to_root(skill.path.parent, entry.source_root)), + ) + for side, other in pairs: + side.findings.append( + Finding( + "warn", + "description-near-duplicate", + f"description overlaps {other} (word overlap {similarity:.0%}); " + "the model may pick either", + ) + ) + + def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillReview]]: """One :class:`SkillReview` per entry, in order. None for an entry that did not load, unless its frontmatter is broken YAML: then its review carries the ``frontmatter-parse`` error. Of each identical ``SKILL.md``, the copy nearest the root is the original; every other copy names it in ``duplicate_of``. + Mirror drift and near-duplicate descriptions are checked across all the loaded skills, + and a skill with ``disable-model-invocation`` set is left out of the description checks. """ reviews: list[Optional[SkillReview]] = [] copies: list[tuple[SkillReview, str, str]] = [] # (review, skill directory, digest) per loaded skill + loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]] = [] for entry in entries: if not entry.loaded or entry.skill is None: reviews.append(_unloaded_review(entry)) @@ -202,10 +403,14 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe skill, at_root=skill.path.parent.resolve() == entry.source_root.resolve() ) review = SkillReview(findings=findings, disable_model_invocation=hidden) + review.findings += _bundle_findings(skill, entry.source_root) + if not hidden: + review.findings += _description_findings(skill) review.content_sha256 = _skill_digest(skill) where = _relative_to_root(skill.path.parent, entry.source_root) copies.append((review, where, review.content_sha256)) reviews.append(review) + loaded.append((entry, skill, review)) original_of: dict[str, tuple[SkillReview, str]] = {} for copy, where, digest in sorted(copies, key=lambda c: (len(PurePosixPath(c[1]).parts), c[1])): @@ -222,4 +427,6 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe f"SKILL.md is identical to {where}; the same skill is counted twice", ) ) + _mirror_drift(loaded) + _near_duplicates(loaded) return reviews diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 2313223a6..e07307147 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -81,8 +81,9 @@ def test_name_missing_from_frontmatter_is_a_warning(tmp_path): def test_description_length_limit_is_1024_characters(tmp_path): - _skill(tmp_path / "skills", "at-limit", name="at-limit", description="d" * 1024) - _skill(tmp_path / "skills", "over-limit", name="over-limit", description="d" * 1025) + # "Use for " keeps a when-clause in the description, so only the length is in question. + _skill(tmp_path / "skills", "at-limit", name="at-limit", description="Use for " + "d" * 1016) + _skill(tmp_path / "skills", "over-limit", name="over-limit", description="Use for " + "d" * 1017) reviews = _reviews_by_name(tmp_path) assert reviews["at-limit"].state == "ok" @@ -114,7 +115,8 @@ def test_different_skill_md_with_the_same_name_is_not_a_duplicate(tmp_path): reviews = review_skills(read_skill_manifest(tmp_path)) assert all(r.duplicate_of is None for r in reviews) - assert all(r.state == "ok" for r in reviews) + # Not a duplicate, but a mirror that has drifted: both copies say so. + assert [[f.code for f in r.findings] for r in reviews] == [["mirror-drift"], ["mirror-drift"]] def test_disable_model_invocation_is_reported_as_a_flag_not_a_finding(tmp_path): @@ -491,3 +493,198 @@ def load(path): assert states["a"] == ("ok", None) assert states["b"][0] == "error" and "Permission denied" in states["b"][1] assert code == 1 + + + +def _codes(review) -> list[str]: + return [f.code for f in review.findings] + + +def test_relative_link_to_a_file_that_is_not_there_is_a_warning(tmp_path): + _skill(tmp_path / "skills", "guided", name="guided", description="Use for guided work.", + body="See [the guide](references/guide.md).\n") + bundled = _skill( + tmp_path / "skills", + "bundled", + name="bundled", + description="Use for bundled work.", + body="Details in [the guide](references/guide.md).\n", + ) + (bundled / "references").mkdir() + (bundled / "references" / "guide.md").write_text("# G\n", encoding="utf-8") + + reviews = _reviews_by_name(tmp_path) + assert reviews["guided"].state == "warn" + assert _codes(reviews["guided"]) == ["link-missing"] + assert reviews["bundled"].state == "ok" + + +def test_link_that_exists_from_the_catalog_root_says_the_base_is_wrong(tmp_path): + (tmp_path / "scripts").mkdir() + (tmp_path / "scripts" / "run.py").write_text("print(1)\n", encoding="utf-8") + _skill(tmp_path / "skills", "runner", name="runner", body="Run [it](scripts/run.py).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + [finding] = review.findings + assert finding.code == "link-missing" + assert "the base is wrong" in finding.message + + +def test_links_in_code_fences_urls_and_anchors_are_not_checked(tmp_path): + body = ( + "Read [the site](https://example.com/docs) or [below](#usage).\n" + "```markdown\n" + "See [an example](references/missing.md).\n" + "```\n" + "~~~\n" + "[also](scripts/missing.sh)\n" + "~~~\n" + ) + _skill(tmp_path / "skills", "clean", name="clean", body=body) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "ok" + assert review.findings == [] + + +def test_skill_dir_variable_resolves_inside_the_skill(tmp_path): + skill = _skill(tmp_path / "skills", "vars", name="vars", + body="[a](${CLAUDE_SKILL_DIR}/references/a.md) [b](${CLAUDE_SKILL_DIR}/b.md)\n") + (skill / "references").mkdir() + (skill / "references" / "a.md").write_text("# A\n", encoding="utf-8") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-missing"] + assert "'${CLAUDE_SKILL_DIR}/b.md'" in review.findings[0].message + + +def test_link_out_of_the_skill_dir_to_an_existing_file_is_a_warning(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "reach", name="reach", body="See [notes](../../shared/notes.md).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-escapes-skill"] + + +def test_link_out_of_the_skill_dir_to_a_missing_file_is_a_warning(tmp_path): + _skill(tmp_path / "skills", "reach", name="reach", body="See [notes](../../nowhere.md).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-missing"] + + +def test_dotdot_code_span_to_an_existing_file_outside_the_skill_is_a_warning(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill( + tmp_path / "skills", + "reach", + name="reach", + body="Keep `../../shared/notes.md` next to it. Edit `docs/missing.md` in your project.\n", + ) + + [review] = review_skills(read_skill_manifest(tmp_path)) + # The code span naming a missing project path is not checked: only the escape is reported. + assert review.state == "warn" + assert _codes(review) == ["path-escapes-skill"] + + +def test_description_without_a_when_clause_is_a_warning(tmp_path): + _skill( + tmp_path / "skills", "plain", name="plain", description="Builds release reports from logs." + ) + _skill(tmp_path / "skills", "said", name="said", + description="Summarises deploy outcomes. Use when a release is cut.") + + reviews = _reviews_by_name(tmp_path) + assert reviews["plain"].state == "warn" + assert _codes(reviews["plain"]) == ["description-no-when"] + assert reviews["said"].state == "ok" + + +def test_hidden_skill_is_exempt_from_the_description_checks(tmp_path): + _skill(tmp_path / "skills", "hidden", name="hidden", description="Builds release reports.", + extra="disable-model-invocation: true\n") + _skill(tmp_path / "skills", "twin", name="twin", description="Builds release reports.", + extra="disable-model-invocation: true\n") + + reviews = _reviews_by_name(tmp_path) + assert reviews["hidden"].state == "ok" + assert reviews["twin"].state == "ok" + + +def test_near_duplicate_descriptions_warn_on_both_skills(tmp_path): + _skill(tmp_path / "skills", "style-check", name="style-check", + description="Use when reviewing Python pull requests for style and bugs.") + _skill(tmp_path / "skills", "style-lint", name="style-lint", + description="Use when reviewing Python pull requests for style and typing.") + + reviews = _reviews_by_name(tmp_path) + assert _codes(reviews["style-check"]) == ["description-near-duplicate"] + assert _codes(reviews["style-lint"]) == ["description-near-duplicate"] + assert "skills/style-lint" in reviews["style-check"].findings[0].message + + +def test_unrelated_descriptions_are_not_near_duplicates(tmp_path): + _skill(tmp_path / "skills", "deploy", name="deploy", + description="Use when deploying containers to Kubernetes clusters.") + _skill(tmp_path / "skills", "notes", name="notes", + description="Use when writing release notes for a changelog.") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert all(r.state == "ok" for r in reviews) + + +def test_drifted_copies_of_one_skill_warn_on_both_and_name_each_other(tmp_path): + _skill(tmp_path / "plugins", "x", name="x", description="Use for x.", body="# Old\n") + _skill(tmp_path / "skills", "x", name="x", description="Use for x.", body="# New\n") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert all(r.duplicate_of is None for r in reviews) + assert [_codes(r) for r in reviews] == [["mirror-drift"], ["mirror-drift"]] + assert "differs from plugins/x" in reviews[1].findings[0].message + assert "differs from skills/x" in reviews[0].findings[0].message + + +def test_identical_copies_are_a_duplicate_and_not_drift(tmp_path): + _skill(tmp_path / "plugins", "x", name="x", description="Use for x.") + _skill(tmp_path / "skills", "x", name="x", description="Use for x.") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert [_codes(r) for r in reviews if r.duplicate_of is not None] == [["duplicate"]] + assert all("mirror-drift" not in _codes(r) for r in reviews) + + +def test_new_warnings_do_not_fail_the_scan_even_under_strict(tmp_path, capsys): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "reach", name="reach", description="Builds reports.", + body="See [notes](../../shared/notes.md).\n") + report, code = _scan_json(tmp_path, capsys, strict=True) + assert code == 0 + assert report["skills"][0]["state"] == "warn" + assert {f["code"] for f in report["skills"][0]["findings"]} == { + "description-no-when", "link-escapes-skill"} + + _skill(tmp_path / "skills", "example", name="example", description="Builds reports.", + body="Format: [Title](URL)\n") + report, code = _scan_json(tmp_path, capsys, strict=True) + assert code == 0 + states = {row["name"]: row["state"] for row in report["skills"]} + assert states == {"reach": "warn", "example": "warn"} + + +def test_the_scan_sees_a_mirror_that_discover_skills_leaves_out(tmp_path): + # discover_skills counts a byte-identical mirror once; the catalog review must still see both. + _skill(tmp_path / "plugins", "x", name="x", description="Use for x.") + _skill(tmp_path / "skills", "x", name="x", description="Use for x.") + assert [s.path.parent.relative_to(tmp_path).as_posix() for s in discover_skills(tmp_path)] == [ + "plugins/x" + ] + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert [r.duplicate_of for r in reviews] == [None, "plugins/x"] From fffcdcf4db5f98c9b94cd5078e41e9fa83aab4e5 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Fri, 30 Oct 2026 22:10:10 -0700 Subject: [PATCH 02/22] fix(catalog): a link target the OS refuses is a missing link, not a crash A relative link whose path has a component over 255 bytes made Path.exists() raise OSError on Python 3.11 to 3.13 (3.14 returns False), so one such skill aborted catalog scan with a traceback and listed no skill. Treat a target the OS refuses as missing: link-missing warn, as for any absent file. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 26 +++++++++++++++++++++----- dokimi/tests/test_catalog_validate.py | 13 +++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 3b1c1f518..14be91c8f 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -251,6 +251,22 @@ def _file_ref(raw: str) -> Optional[str]: return text +def _resolved(path: Path) -> Optional[Path]: + """``path`` resolved, or None when the OS refuses the name (a NUL byte, a symlink loop).""" + try: + return path.resolve() + except (OSError, RuntimeError, ValueError): + return None + + +def _exists(path: Path) -> bool: + """``path.exists()``, where a name the OS refuses (over 255 bytes in one part, say) is absent.""" + try: + return path.exists() + except (OSError, ValueError): + return False + + def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: """Link and path checks on the body of ``skill``: missing targets, or targets outside it.""" skill_dir = skill.path.parent @@ -268,8 +284,8 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: text = _file_ref(raw) if text is None: continue - target = (skill_dir / text).resolve() - if target.exists(): + target = _resolved(skill_dir / text) + if target is not None and _exists(target): if not target.is_relative_to(real_dir): findings.append( Finding( @@ -281,7 +297,7 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: ) continue from_root = "" - if (source_root / text).exists(): + if _exists(source_root / text): from_root = " (it exists from the catalog root: the base is wrong)" findings.append( Finding( @@ -294,8 +310,8 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: text = _file_ref(raw) if text is None: continue - target = (skill_dir / text).resolve() - if target.exists() and not target.is_relative_to(real_dir): + target = _resolved(skill_dir / text) + if target is not None and _exists(target) and not target.is_relative_to(real_dir): findings.append( Finding( "warn", diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index e07307147..66e8c3bd6 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -688,3 +688,16 @@ def test_the_scan_sees_a_mirror_that_discover_skills_leaves_out(tmp_path): reviews = review_skills(read_skill_manifest(tmp_path)) assert [r.duplicate_of for r in reviews] == [None, "plugins/x"] + + +def test_a_link_target_the_os_refuses_is_a_missing_link_not_a_crash(tmp_path): + # A name over 255 bytes in one path part: Path.exists() raises on Python 3.11 to 3.13. + too_long = "a" * 300 + _skill(tmp_path / "skills", "healthy", name="healthy", description="Use for healthy.", + body="Run [it](references/a.md).\n") + _skill(tmp_path / "skills", "longname", name="longname", description="Use for longname.", + body=f"See [g]({too_long}.md).\n") + + reviews = _reviews_by_name(tmp_path) + assert reviews["healthy"].state == "warn" + assert _codes(reviews["longname"]) == ["link-missing"] From 1e58aa733a8b4aa515728c5fd99707db065b4608 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sat, 31 Oct 2026 12:38:49 -0700 Subject: [PATCH 03/22] Address review: bundle links skip URIs and read reference and angle-bracket targets Thread: PRRT_kwDOVBg7g86yORfR Thread: PRRT_kwDOVBg7g86yORfU Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 21 ++++++-- dokimi/tests/test_catalog_validate.py | 70 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 14be91c8f..3d22d2d88 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -32,7 +32,9 @@ check in :mod:`dokimi.harnesses.skill_treatment`). Bundle links: a relative Markdown link in ``SKILL.md`` whose file does not exist is a -``warn`` (``link-missing``), with a note when the file exists from the catalog root +``warn`` (``link-missing``), inline or reference-style (``[label]: target``), with an +optional ``<...>`` destination, and URI destinations (``tel:``, ``urn:``, ``mailto:``) are +not files. A note is added when the file exists from the catalog root instead (a wrong base). Example links such as ``[Title](URL)`` are common, so this is not an error and does not fail ``--strict``. A link to a file outside the skill directory is a ``warn`` (``link-escapes-skill``): it breaks when the skill is installed alone. A ``../`` code span @@ -87,6 +89,9 @@ just""".split() ) LINK_RE = re.compile(r"\]\(([^)\s]+)(?:\s+\"[^\"]*\")?\)") +# A reference-style link definition, ``[label]: destination`` (footnotes ``[^n]:`` excluded). +REF_DEF_RE = re.compile(r"^ {0,3}\[(?!\^)[^\]]+\]:[ \t]*(\S+)") +URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+.\-]*:") CODE_SPAN_RE = re.compile(r"`([^`\n]+)`") SKILL_DIR_VAR = "${CLAUDE_SKILL_DIR}/" @@ -238,11 +243,14 @@ def _outside_fences(body: str) -> list[str]: def _file_ref(raw: str) -> Optional[str]: """The skill-relative file a reference names, or None if it names no file to check. - None for a URL, a bare anchor, an absolute path, or a template with a variable other - than ``${CLAUDE_SKILL_DIR}``. + None for a URL or any other URI (``tel:``, ``urn:``, ``MAILTO:``), a bare anchor, an + absolute path, or a template with a variable other than ``${CLAUDE_SKILL_DIR}``. A + ``<...>`` destination (CommonMark) is read as the path inside the brackets. """ text = raw.split("#", 1)[0].strip() - if not text or "://" in text or text.startswith(("mailto:", "/")): + if text.startswith("<") and text.endswith(">"): + text = text[1:-1].strip() + if not text or "://" in text or URI_SCHEME_RE.match(text) or text.startswith("/"): return None if text.startswith(SKILL_DIR_VAR): return text[len(SKILL_DIR_VAR) :] or None @@ -272,7 +280,10 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: skill_dir = skill.path.parent real_dir = skill_dir.resolve() lines = _outside_fences(skill.body) - links = dict.fromkeys(m for line in lines for m in LINK_RE.findall(line)) + links = dict.fromkeys( + [m for line in lines for m in LINK_RE.findall(line)] + + [m for line in lines for m in REF_DEF_RE.findall(line)] + ) spans = dict.fromkeys( s.strip() for line in lines diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 66e8c3bd6..ab431ada0 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -701,3 +701,73 @@ def test_a_link_target_the_os_refuses_is_a_missing_link_not_a_crash(tmp_path): reviews = _reviews_by_name(tmp_path) assert reviews["healthy"].state == "warn" assert _codes(reviews["longname"]) == ["link-missing"] + + +@pytest.mark.parametrize( + "destination", + ["tel:+15551234567", "urn:isbn:0451450523", "MAILTO:someone@example.com", "data:text/plain,x"], +) +def test_uri_scheme_destinations_are_not_files(tmp_path, destination): + _skill(tmp_path / "skills", "uris", name="uris", description="Use for uris.", + body=f"Reach us at [here]({destination}).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "ok" + assert review.findings == [] + + +def test_reference_definition_to_a_missing_file_is_a_warning(tmp_path): + _skill(tmp_path / "skills", "refs", name="refs", description="Use for refs.", + body="See [the guide][g] for details.\n\n[g]: references/missing.md\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-missing"] + + +def test_reference_definition_out_of_the_skill_dir_is_a_warning(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "reach", name="reach", description="Use for reach.", + body="See [the notes][n].\n\n[n]: ../../shared/notes.md \"Notes\"\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-escapes-skill"] + + +def test_reference_definitions_that_name_no_file_are_not_checked(tmp_path): + body = ( + "A footnote[^1] and [a site][web].\n\n" + "[^1]: Footnote text, not a file.\n" + "[web]: https://example.com/docs\n" + "[anchor]: #usage\n" + "```markdown\n" + "[example]: references/missing.md\n" + "```\n" + ) + _skill(tmp_path / "skills", "clean", name="clean", description="Use for clean.", body=body) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "ok" + assert review.findings == [] + + +def test_angle_bracket_destination_to_a_missing_file_is_a_warning(tmp_path): + _skill(tmp_path / "skills", "angle", name="angle", description="Use for angle.", + body="See [the guide]().\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-missing"] + + +def test_angle_bracket_destination_out_of_the_skill_dir_is_a_warning(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "reach", name="reach", description="Use for reach.", + body="See [notes](<../../shared/notes.md>).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-escapes-skill"] From cae82c0fd7a93ed89cc2d9625b8c3b99b24b02db Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sat, 31 Oct 2026 12:40:36 -0700 Subject: [PATCH 04/22] Address review: parse reference definitions and <...> targets as CommonMark does A reference definition needs an end of line after its destination and an optional title, so prose such as '[Note]: read this first' is not one. The <...> unwrap runs before the #-anchor split, so an anchored angle-bracket target is checked. Thread: PRRT_kwDOVBg7g86yORfR Thread: PRRT_kwDOVBg7g86yORfU Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 11 +++++-- dokimi/tests/test_catalog_validate.py | 43 +++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 3d22d2d88..5123bafbe 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -90,8 +90,12 @@ ) LINK_RE = re.compile(r"\]\(([^)\s]+)(?:\s+\"[^\"]*\")?\)") # A reference-style link definition, ``[label]: destination`` (footnotes ``[^n]:`` excluded). -REF_DEF_RE = re.compile(r"^ {0,3}\[(?!\^)[^\]]+\]:[ \t]*(\S+)") -URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+.\-]*:") +REF_DEF_RE = re.compile( + r"""^ {0,3}\[(?!\^)[^\]]+\]:[ \t]*(<[^>\n]*>|\S+)""" + r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""" +) +# A scheme has no dot, so ``README.md:42`` and ``run.py:main`` stay paths to check. +URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-]*:") CODE_SPAN_RE = re.compile(r"`([^`\n]+)`") SKILL_DIR_VAR = "${CLAUDE_SKILL_DIR}/" @@ -247,9 +251,10 @@ def _file_ref(raw: str) -> Optional[str]: absolute path, or a template with a variable other than ``${CLAUDE_SKILL_DIR}``. A ``<...>`` destination (CommonMark) is read as the path inside the brackets. """ - text = raw.split("#", 1)[0].strip() + text = raw.strip() if text.startswith("<") and text.endswith(">"): text = text[1:-1].strip() + text = text.split("#", 1)[0].strip() if not text or "://" in text or URI_SCHEME_RE.match(text) or text.startswith("/"): return None if text.startswith(SKILL_DIR_VAR): diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index ab431ada0..2909db9f2 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -771,3 +771,46 @@ def test_angle_bracket_destination_out_of_the_skill_dir_is_a_warning(tmp_path): [review] = review_skills(read_skill_manifest(tmp_path)) assert review.state == "warn" assert _codes(review) == ["link-escapes-skill"] + + +def test_angle_bracket_destination_with_an_anchor_out_of_the_skill_dir_is_a_warning(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "reach", name="reach", description="Use for reach.", + body="See [setup](<../../shared/notes.md#setup>).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-escapes-skill"] + + +def test_reference_definition_with_spaces_in_angle_brackets_to_a_missing_file_is_a_warning(tmp_path): + _skill(tmp_path / "skills", "spaced", name="spaced", description="Use for spaced.", + body="See [notes][n].\n\n[n]: \n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-missing"] + + +@pytest.mark.parametrize( + "line", + ["[Note]: read this first", "[Step 1]: Run the tests", "[--verbose]: print more"], +) +def test_prose_line_that_starts_with_a_bracketed_label_is_not_a_definition(tmp_path, line): + _skill(tmp_path / "skills", "prose", name="prose", description="Use for prose.", + body=f"Start here.\n\n{line}\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "ok" + assert review.findings == [] + + +def test_a_file_line_link_to_a_missing_file_is_still_a_warning(tmp_path): + # "run.py:42" is read as a path, not a URI, so it is checked and the missing file is flagged. + _skill(tmp_path / "skills", "fileline", name="fileline", description="Use for fileline.", + body="See [the entry point](run.py:42).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert _codes(review) == ["link-missing"] From 8c07f95ed8cb550632a6e781fb7ee9df0d8fe5d8 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sat, 31 Oct 2026 17:58:33 -0700 Subject: [PATCH 05/22] test(reference): pin the shared regex limit with a fake clock, not wall time The shared-limit test compared elapsed time against 1.5x REGEX_TIMEOUT and flaked on the loaded runner. A count of searches alone did not pin the shared budget: a per-pattern budget left the same count. A fake clock now charges each search 0.6x the limit, so the second pattern can only finish with a fresh budget, which the shared deadline does not give it. Signed-off-by: Nikolay Petrov --- dokimi/tests/test_reference_checks.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/dokimi/tests/test_reference_checks.py b/dokimi/tests/test_reference_checks.py index d9a18b734..e1fd5d2e3 100644 --- a/dokimi/tests/test_reference_checks.py +++ b/dokimi/tests/test_reference_checks.py @@ -14,10 +14,13 @@ import json import time +from types import SimpleNamespace import pytest +import regex from dokimi.artifacts import save_artifact +from dokimi.eval import reference_checks from dokimi.eval.checklist import ASK_COVERED from dokimi.eval.reference_checks import ( REGEX_TIMEOUT, chosen_letters, grade, match_text, parse_json, validate_grading, @@ -189,11 +192,27 @@ def test_an_all_mode_regex_is_not_settled_by_its_first_match(): assert _one({"type": "regex", "values": ["a", "zzz"], "mode": "all"}, "a").passed is False -def test_the_regex_limit_is_shared_by_every_pattern_of_one_check(): - started = time.monotonic() - g = _one({"type": "regex", "values": ["(a|aa)+$", "(a|aa)+$"]}, "a" * 40 + "!") +def test_the_regex_limit_is_shared_by_every_pattern_of_one_check(monkeypatch): + # One budget for the whole check, not one per pattern. A fake clock stands in for wall time: + # each search costs 0.6 * REGEX_TIMEOUT, so the second pattern has only 0.4 left and must time + # out, while a per-pattern budget would let it finish. Asserts on the work done, not the clock. + now = [0.0] + monkeypatch.setattr(reference_checks, "time", SimpleNamespace(monotonic=lambda: now[0])) + searched = [] + + def search(pattern, string, flags=0, timeout=None): + searched.append(pattern) + cost = 0.6 * REGEX_TIMEOUT + if cost > timeout: + now[0] += timeout + raise TimeoutError("regex search ran past its timeout") + now[0] += cost + return object() # a match + + monkeypatch.setattr(regex, "search", search) + g = _one({"type": "regex", "values": ["p1", "p2"], "mode": "all"}, "answer") assert g.checks[0].passed is None - assert time.monotonic() - started < 1.5 * REGEX_TIMEOUT + assert searched == ["p1", "p2"] def test_a_question_with_one_failing_check_fails_without_a_threshold(): From ae8cad9c4147c68701019f37b5b35065358ca703 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sat, 31 Oct 2026 18:07:16 -0700 Subject: [PATCH 06/22] test(reference): keep the shared-limit comment to two lines The comment-block check allows two lines of reason per comment. Signed-off-by: Nikolay Petrov --- dokimi/tests/test_reference_checks.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dokimi/tests/test_reference_checks.py b/dokimi/tests/test_reference_checks.py index e1fd5d2e3..0b72ed309 100644 --- a/dokimi/tests/test_reference_checks.py +++ b/dokimi/tests/test_reference_checks.py @@ -193,9 +193,8 @@ def test_an_all_mode_regex_is_not_settled_by_its_first_match(): def test_the_regex_limit_is_shared_by_every_pattern_of_one_check(monkeypatch): - # One budget for the whole check, not one per pattern. A fake clock stands in for wall time: - # each search costs 0.6 * REGEX_TIMEOUT, so the second pattern has only 0.4 left and must time - # out, while a per-pattern budget would let it finish. Asserts on the work done, not the clock. + # One budget per check: a fake clock charges each search 0.6x the limit, so the second + # pattern has only 0.4x left and times out (a per-pattern budget would let it finish). now = [0.0] monkeypatch.setattr(reference_checks, "time", SimpleNamespace(monotonic=lambda: now[0])) searched = [] From 3f40580f53995f496bd024364f708e1c9aa777bf Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 13:58:29 +0000 Subject: [PATCH 07/22] Compare only skills that share a word in the near-duplicate pass Signed-off-by: Anton Volkov --- dokimi/dokimi/external/validate.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 5123bafbe..9e0566dd4 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -390,8 +390,13 @@ def _near_duplicates(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) """``description-near-duplicate`` on both skills of each pair whose descriptions overlap.""" shown = [item for item in loaded if not item[2].disable_model_invocation] tokens = [_tokens(" ".join(item[1].description.split())) for item in shown] + by_token: dict[str, list[int]] = {} + for index, words in enumerate(tokens): + for word in words: + by_token.setdefault(word, []).append(index) for i, (entry, skill, review) in enumerate(shown): - for j in range(i + 1, len(shown)): + # only a pair that shares a word can overlap + for j in sorted({k for word in tokens[i] for k in by_token[word] if k > i}): o_entry, o_skill, o_review = shown[j] if o_review.content_sha256 == review.content_sha256: continue # an identical SKILL.md: already a duplicate From e1afc3c21068349b658ebdf09aa439c344551be0 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 13:58:59 +0000 Subject: [PATCH 08/22] Pin the near-duplicate skips and the 0.5 overlap threshold in tests Signed-off-by: Anton Volkov --- dokimi/tests/test_catalog_validate.py | 33 +++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 2909db9f2..9d053fb42 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -639,6 +639,39 @@ def test_unrelated_descriptions_are_not_near_duplicates(tmp_path): assert all(r.state == "ok" for r in reviews) +def test_identical_and_same_named_pairs_get_no_near_duplicate_warning(tmp_path): + same = "Use when reviewing Python pull requests for style and bugs." + _skill(tmp_path / "skills", "copy-a", name="copy", description=same) + _skill(tmp_path / "skills", "copy-b", name="copy", description=same) + other = "Builds Kubernetes container deployment manifests." + _skill(tmp_path / "plugins", "x", name="x", description=other, body="# Old\n") + _skill(tmp_path / "skills", "x", name="x", description=other, body="# New\n") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert len(reviews) == 4 + assert all("description-near-duplicate" not in _codes(r) for r in reviews) + + +def _words(prefix: str, count: int) -> str: + return " ".join(f"{prefix}{c}" for c in "abcdefghijklmnopqrstuvwxyz"[:count]) + + +@pytest.mark.parametrize( + ("shared", "own_a", "own_b", "warns"), + [(2, 1, 1, True), (24, 13, 12, False)], # 2/4 = 0.5 exactly; 24/49 just below it +) +def test_near_duplicate_threshold_is_half_the_words(tmp_path, shared, own_a, own_b, warns): + common = _words("zq", shared) + _skill(tmp_path / "skills", "one", name="one", + description=f"{common} {_words('xa', own_a)}") + _skill(tmp_path / "skills", "two", name="two", + description=f"{common} {_words('xb', own_b)}") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert ("description-near-duplicate" in _codes(reviews[0])) is warns + assert ("description-near-duplicate" in _codes(reviews[1])) is warns + + def test_drifted_copies_of_one_skill_warn_on_both_and_name_each_other(tmp_path): _skill(tmp_path / "plugins", "x", name="x", description="Use for x.", body="# Old\n") _skill(tmp_path / "skills", "x", name="x", description="Use for x.", body="# New\n") From e4dc40456dd255c7e34dfb8134caa6176198af6d Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 09:26:22 -0800 Subject: [PATCH 09/22] docs(changelog): qualify the exit-code note for spec and load errors Load errors fail without --strict, so the note now says which errors fail only under --strict. Thread: PRRT_kwDOVBg7g86ybI3b Signed-off-by: Nikolay Petrov --- .../20261030_170000_napetrov_catalog-bundle-drift-checks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md b/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md index b078b8b9b..41b688714 100644 --- a/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md +++ b/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md @@ -11,4 +11,4 @@ a `warn` on both (`mirror-drift`), naming the other copies. Links inside code fences, URLs, anchors, and code spans that name no file are not checked. Skills with `disable-model-invocation: true` are exempt from the description checks. Warnings never - change the exit code; only `--strict` makes an `error` fail. + change the exit code; a spec `error` fails only with `--strict`, while a load error always fails. From 3b270ea12b381141dd79c1d5e7b87247814d5c24 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 09:26:22 -0800 Subject: [PATCH 10/22] fix(catalog): read dotted URI schemes, template variables, root skills and non-ASCII words - A dotted RFC 3986 scheme (z39.50r:example.org/database) is a URI, not a file. A dotted scheme followed by a line number or symbol (run.py:42, run.py:main) is still a file reference. - A ${CLAUDE_SKILL_DIR}/ path that also holds another variable is not checked as a file; the prefix no longer skips the variable check. - The root SKILL.md is named by the checkout's directory, so it is not grouped with a same-named skill for mirror-drift or the near-duplicate skip. - Description words are any letters, not only ASCII, so a non-English description overlaps other skills. Thread: PRRT_kwDOVBg7g86ybCa1 Thread: PRRT_kwDOVBg7g86ybI3p Thread: PRRT_kwDOVBg7g86ybI3s Thread: PRRT_kwDOVBg7g86ybI3y Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 39 +++++++++++++++++----- dokimi/tests/test_catalog_validate.py | 48 +++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 9e0566dd4..b5da20a6a 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -94,8 +94,11 @@ r"""^ {0,3}\[(?!\^)[^\]]+\]:[ \t]*(<[^>\n]*>|\S+)""" r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""" ) -# A scheme has no dot, so ``README.md:42`` and ``run.py:main`` stay paths to check. -URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-]*:") +# RFC 3986 scheme: letters, digits, ``+``, ``-`` and ``.`` (``z39.50r:``). A dotted scheme is +# still read as a path when what follows the colon is a line number or a symbol, the +# ``README.md:42`` and ``run.py:main`` file references. +URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-.]*:") +FILE_SUFFIX_RE = re.compile(r"\d+|[A-Za-z_]\w*") CODE_SPAN_RE = re.compile(r"`([^`\n]+)`") SKILL_DIR_VAR = "${CLAUDE_SKILL_DIR}/" @@ -255,10 +258,15 @@ def _file_ref(raw: str) -> Optional[str]: if text.startswith("<") and text.endswith(">"): text = text[1:-1].strip() text = text.split("#", 1)[0].strip() - if not text or "://" in text or URI_SCHEME_RE.match(text) or text.startswith("/"): + if not text or "://" in text or text.startswith("/"): + return None + scheme = URI_SCHEME_RE.match(text) + if scheme and not ("." in scheme.group() and FILE_SUFFIX_RE.fullmatch(text[scheme.end() :])): return None if text.startswith(SKILL_DIR_VAR): - return text[len(SKILL_DIR_VAR) :] or None + text = text[len(SKILL_DIR_VAR) :] + if not text: + return None if any(c in text for c in "${}<>*"): return None return text @@ -354,8 +362,8 @@ def _description_findings(skill: Skill) -> list[Finding]: def _tokens(description: str) -> frozenset[str]: - """The lowercase words of three or more letters in ``description``, less the stop words.""" - words = re.findall(r"[a-z]{3,}", description.lower()) + """The lowercase words of three or more letters, in any script, less the stop words.""" + words = re.findall(r"[^\W\d_]{3,}", description.lower()) return frozenset(w for w in words if w not in STOP_WORDS) @@ -363,11 +371,25 @@ def _jaccard(a: frozenset[str], b: frozenset[str]) -> float: return len(a & b) / len(a | b) if a and b else 0.0 +def _dir_name(item: tuple[SkillCatalogEntry, Skill, SkillReview]) -> Optional[str]: + """The directory name that can make a skill a mirror of another, None for the root skill. + + The root ``SKILL.md`` is named by the checkout's directory, not by the skill, so it has no + name to match (as ``_frontmatter_findings`` treats it with ``at_root``). + """ + entry, skill, _ = item + if skill.path.parent.resolve() == entry.source_root.resolve(): + return None + return skill.path.parent.name + + def _mirror_drift(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) -> None: """``mirror-drift`` on each skill that shares its directory name with a differing copy.""" by_dir_name: dict[str, list[tuple[SkillCatalogEntry, Skill, SkillReview]]] = {} for item in loaded: - by_dir_name.setdefault(item[1].path.parent.name, []).append(item) + name = _dir_name(item) + if name is not None: + by_dir_name.setdefault(name, []).append(item) for group in by_dir_name.values(): for _, _, review in group: others = [ @@ -390,6 +412,7 @@ def _near_duplicates(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) """``description-near-duplicate`` on both skills of each pair whose descriptions overlap.""" shown = [item for item in loaded if not item[2].disable_model_invocation] tokens = [_tokens(" ".join(item[1].description.split())) for item in shown] + names = [_dir_name(item) for item in shown] by_token: dict[str, list[int]] = {} for index, words in enumerate(tokens): for word in words: @@ -400,7 +423,7 @@ def _near_duplicates(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) o_entry, o_skill, o_review = shown[j] if o_review.content_sha256 == review.content_sha256: continue # an identical SKILL.md: already a duplicate - if o_skill.path.parent.name == skill.path.parent.name: + if names[i] is not None and names[i] == names[j]: continue # the same skill in another path: already a mirror-drift similarity = _jaccard(tokens[i], tokens[j]) if similarity >= NEAR_DUPLICATE_JACCARD: diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 9d053fb42..760203c40 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -847,3 +847,51 @@ def test_a_file_line_link_to_a_missing_file_is_still_a_warning(tmp_path): [review] = review_skills(read_skill_manifest(tmp_path)) assert review.state == "warn" assert _codes(review) == ["link-missing"] + + +@pytest.mark.parametrize( + "destination", ["z39.50r:example.org/database", "x.y+z:records/item"], +) +def test_dotted_uri_scheme_destinations_are_not_files(tmp_path, destination): + _skill(tmp_path / "skills", "dotted", name="dotted", description="Use for dotted.", + body=f"Look up [the record]({destination}).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +def test_skill_dir_variable_with_another_variable_is_not_read_as_a_file(tmp_path): + _skill(tmp_path / "skills", "tpl", name="tpl", description="Use for tpl.", + body="Run [it](${CLAUDE_SKILL_DIR}/scripts/${SCRIPT_NAME}.py).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +def test_root_skill_and_a_nested_skill_named_like_the_checkout_are_not_mirror_drift(tmp_path): + # The root SKILL.md is named by the checkout's directory, so it is no copy of skills/tools. + root = tmp_path / "tools" + _root_skill(root, "name: root-skill\ndescription: Use for root work.\n") + _skill(root, "skills/tools", name="tools", description="Use for tool work.") + + reviews = _reviews_by_name(root) + assert _codes(reviews["root-skill"]) == [] + assert _codes(reviews["tools"]) == [] + + +def test_near_duplicate_check_reads_non_ascii_description_words(tmp_path): + shared = "Use when проверка исходного кода необходима." + _skill(tmp_path / "skills", "code-check", name="code-check", description=shared) + _skill(tmp_path / "skills", "code-audit", name="code-audit", description=shared) + + reviews = _reviews_by_name(tmp_path) + assert _codes(reviews["code-check"]) == ["description-near-duplicate"] + assert _codes(reviews["code-audit"]) == ["description-near-duplicate"] + + +def test_a_file_symbol_link_to_a_missing_file_is_still_a_warning(tmp_path): + _skill(tmp_path / "skills", "symbol", name="symbol", description="Use for symbol.", + body="See [the entry point](run.py:main).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-missing"] From e7597421d2320d74dd31d16b97efe1846bbdaa65 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 09:55:09 -0800 Subject: [PATCH 11/22] style(catalog): fit the URI scheme comment in two lines The Comment blocks check fails a three-line block of comments. Fixes the Static checks failure on 3b270ea1. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index b5da20a6a..54c404887 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -94,9 +94,7 @@ r"""^ {0,3}\[(?!\^)[^\]]+\]:[ \t]*(<[^>\n]*>|\S+)""" r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""" ) -# RFC 3986 scheme: letters, digits, ``+``, ``-`` and ``.`` (``z39.50r:``). A dotted scheme is -# still read as a path when what follows the colon is a line number or a symbol, the -# ``README.md:42`` and ``run.py:main`` file references. +# RFC 3986 scheme, dots included; a dotted one with a line or symbol after it is a file (_file_ref). URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-.]*:") FILE_SUFFIX_RE = re.compile(r"\d+|[A-Za-z_]\w*") CODE_SPAN_RE = re.compile(r"`([^`\n]+)`") From 99bbe675e96bfaae627313b49ae94248f6e8e9bf Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 11:20:57 -0800 Subject: [PATCH 12/22] fix(catalog): read a line or symbol suffix as part of the file name A link or code span like run.py:42 or run.py:main named a file that does not exist, so an existing bundled file was reported as link-missing and an escape through ../ was hidden. The suffix is now dropped before the target is resolved. A description is checked for any "when" clause, not only a fixed list of phrases, so "Run migrations when deploying" is not a warning. Signed-off-by: Nikolay Petrov --- ...00_napetrov_catalog-bundle-drift-checks.md | 2 +- dokimi/docs/external-content.md | 2 +- dokimi/dokimi/external/validate.py | 24 ++++++----- dokimi/tests/test_catalog_validate.py | 43 +++++++++++++++++++ 4 files changed, 59 insertions(+), 12 deletions(-) diff --git a/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md b/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md index 41b688714..35539f099 100644 --- a/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md +++ b/dokimi/changelog.d/20261030_170000_napetrov_catalog-bundle-drift-checks.md @@ -5,7 +5,7 @@ (`link-missing`), and says so when the file exists from the catalog root instead (a wrong base). A link or `../` code span that reaches a file outside the skill directory is a `warn` (`link-escapes-skill`, `path-escapes-skill`): it breaks when the skill is installed alone. - A description with no "when to use" clause is a `warn` (`description-no-when`), as is one + A description with no "when" clause is a `warn` (`description-no-when`), as is one whose words overlap another skill's by at least half (`description-near-duplicate`). Two skills in different paths with the same directory name and different `SKILL.md` text are a `warn` on both (`mirror-drift`), naming the other copies. Links inside code fences, URLs, diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index 4cc80d246..a61029764 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -135,7 +135,7 @@ file exists from the catalog root instead. Example links such as `[Title](URL)` are common, so `--strict` does not fail on them. A link, or a `../` code span, that reaches a file outside the skill directory is a `warn` (`link-escapes-skill`, `path-escapes-skill`), since it breaks when the skill is -installed alone. A description with no "when to use" clause is a `warn` +installed alone. A description with no "when" clause (`use when ...`, `when deploying ...`) is a `warn` (`description-no-when`), and so is one that overlaps another skill's by at least half (`description-near-duplicate`). Two skills in different paths with the same directory name and different `SKILL.md` text are a `warn` on both diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 54c404887..059990427 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -41,7 +41,7 @@ naming such a file is ``path-escapes-skill``. Code spans that name no file are not checked, and neither are code fences: those are usually examples or paths in the user's own project. -Descriptions: one with no "when to use" clause is a ``warn`` (``description-no-when``), +Descriptions: one with no "when" clause is a ``warn`` (``description-no-when``), and so is one whose tokens overlap another skill's at a Jaccard similarity of at least :data:`NEAR_DUPLICATE_JACCARD` (``description-near-duplicate``). Skills with ``disable-model-invocation: true`` are exempt from both, as the model never chooses them. @@ -71,13 +71,12 @@ DESCRIPTION_MAX = 1024 NEAR_DUPLICATE_JACCARD = 0.5 -# A "when to use" clause: the model chooses a skill from its description alone. +# A "when" clause of any subject and verb ("run migrations when deploying"), or a use phrase: the model +# chooses a skill from its description alone. WHEN_RE = re.compile( - r"\b(?:when to use|when to delegate|use when|use this when|use for|use this skill|" - r"trigger(?:s|ed)?\s*(?:on|when|:)|invoke (?:when|this)|invoke only|" - r"use (?:it |this |these )?(?:when|if|before|after|whenever|proactively|anytime)|" - r"whenever|when the user|activate\b|should be used when|when building|" - r"when (?:you|the user) )", + r"\b(?:when\s+\w|use for|use this skill|trigger(?:s|ed)?\s*(?:on|:)|invoke only|activate\b|" + r"invoke this|use (?:it |this |these )?(?:when|if|before|after|whenever|proactively|anytime)|" + r"whenever)", re.I, ) # Words that say nothing about what a skill is for; left out of the description overlap. @@ -96,7 +95,9 @@ ) # RFC 3986 scheme, dots included; a dotted one with a line or symbol after it is a file (_file_ref). URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-.]*:") -FILE_SUFFIX_RE = re.compile(r"\d+|[A-Za-z_]\w*") +# After a file, a line or a symbol (``run.py:42``, ``run.py:10-20``, ``run.py:main``): read as the file's name. +FILE_SUFFIX_RE = re.compile(r"\d+(?:[-:]\d+)?|[A-Za-z_]\w*") +LOCATION_SUFFIX_RE = re.compile(r":(?:\d+(?:[-:]\d+)?|[A-Za-z_]\w*)$") CODE_SPAN_RE = re.compile(r"`([^`\n]+)`") SKILL_DIR_VAR = "${CLAUDE_SKILL_DIR}/" @@ -261,6 +262,9 @@ def _file_ref(raw: str) -> Optional[str]: scheme = URI_SCHEME_RE.match(text) if scheme and not ("." in scheme.group() and FILE_SUFFIX_RE.fullmatch(text[scheme.end() :])): return None + text = LOCATION_SUFFIX_RE.sub("", text) # the file is named before its line or symbol suffix + if not text: + return None if text.startswith(SKILL_DIR_VAR): text = text[len(SKILL_DIR_VAR) :] if not text: @@ -346,13 +350,13 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: def _description_findings(skill: Skill) -> list[Finding]: - """A ``warn`` when the description has no "when to use" clause.""" + """A ``warn`` when the description has no "when" clause.""" if not WHEN_RE.search(" ".join(skill.description.split())): return [ Finding( "warn", "description-no-when", - "description has no 'when to use' or 'use when' clause; the model chooses " + "description has no 'when' clause (e.g. 'use when ...'); the model chooses " "the skill from the description alone", ) ] diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 760203c40..53627b29d 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -895,3 +895,46 @@ def test_a_file_symbol_link_to_a_missing_file_is_still_a_warning(tmp_path): [review] = review_skills(read_skill_manifest(tmp_path)) assert _codes(review) == ["link-missing"] + + +@pytest.mark.parametrize( + "description", + [ + "Run database migrations when deploying a new release.", + "Diagnose failures when debugging production incidents.", + "Format commit messages when the user asks for a commit.", + ], +) +def test_a_general_when_clause_is_not_a_description_warning(tmp_path, description): + _skill(tmp_path / "skills", "clause", name="clause", description=description) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == [] + + +@pytest.mark.parametrize("body", [ + "See [the entry point](run.py:42).\n", + "See [the entry point](run.py:main).\n", + "See [the entry point](run.py:10-20).\n", + "See [the entry point](scripts/run.py:42:7).\n", +]) +def test_an_existing_file_with_a_line_or_symbol_suffix_is_not_a_missing_link(tmp_path, body): + skill_dir = _skill(tmp_path / "skills", "bundled-run", name="bundled-run", + description="Use for bundled run.", body=body) + (skill_dir / "run.py").write_text("print('x')\n", encoding="utf-8") + (skill_dir / "scripts").mkdir() + (skill_dir / "scripts" / "run.py").write_text("print('x')\n", encoding="utf-8") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +def test_an_escaping_link_with_a_line_suffix_is_named_as_escaping(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "run.py").write_text("print('x')\n", encoding="utf-8") + _skill(tmp_path / "skills", "reach-run", name="reach-run", description="Use for reach run.", + body="See [the entry point](../../shared/run.py:42).\n" + "Also `../../shared/run.py:main`.\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert sorted(_codes(review)) == ["link-escapes-skill", "path-escapes-skill"] From 782412c23e3ee5fbf11fcc359a197fce5366b664 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 20:43:21 +0000 Subject: [PATCH 13/22] test(catalog): pin range and multi-colon suffixes of missing file links Signed-off-by: Anton Volkov --- dokimi/tests/test_catalog_validate.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 53627b29d..7ca82bf1e 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -839,10 +839,11 @@ def test_prose_line_that_starts_with_a_bracketed_label_is_not_a_definition(tmp_p assert review.findings == [] -def test_a_file_line_link_to_a_missing_file_is_still_a_warning(tmp_path): +@pytest.mark.parametrize("destination", ["run.py:42", "run.py:10-20", "scripts/run.py:42:7"]) +def test_a_file_line_link_to_a_missing_file_is_still_a_warning(tmp_path, destination): # "run.py:42" is read as a path, not a URI, so it is checked and the missing file is flagged. _skill(tmp_path / "skills", "fileline", name="fileline", description="Use for fileline.", - body="See [the entry point](run.py:42).\n") + body=f"See [the entry point]({destination}).\n") [review] = review_skills(read_skill_manifest(tmp_path)) assert review.state == "warn" From f5253d4793503b8170e73495004934c0b6027697 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 20:43:29 +0000 Subject: [PATCH 14/22] test(catalog): cover root skills of two checkouts in drift checks Signed-off-by: Anton Volkov --- dokimi/tests/test_catalog_validate.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 7ca82bf1e..11dca3079 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -880,6 +880,24 @@ def test_root_skill_and_a_nested_skill_named_like_the_checkout_are_not_mirror_dr assert _codes(reviews["tools"]) == [] +def test_root_skills_of_two_checkouts_are_near_duplicates_not_mirror_drift(tmp_path): + # Both root skills reached as "sub/.." have no directory name, so they never mirror each other. + entries = [] + for checkout, ending in (("a/repo", "bugs"), ("b/repo", "typing")): + root = tmp_path / checkout + root.mkdir(parents=True) + _root_skill( + root, f"name: r{checkout[0]}\ndescription: Use when reviewing Python pull " + f"requests for style and {ending}.\n") + (root / "sub").mkdir() + (root / "skills.yaml").write_text( + f"skills:\n - name: r{checkout[0]}\n catalog_path: sub/..\n", encoding="utf-8") + entries.extend(read_skill_manifest(root)) + + reviews = review_skills(entries) + assert [_codes(r) for r in reviews] == [["description-near-duplicate"]] * 2 + + def test_near_duplicate_check_reads_non_ascii_description_words(tmp_path): shared = "Use when проверка исходного кода необходима." _skill(tmp_path / "skills", "code-check", name="code-check", description=shared) From 2a6a8cad724bb03164a5a50a750a3c3b3acd8b40 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 13:12:20 -0800 Subject: [PATCH 15/22] fix(catalog): read link targets, definitions and fences as CommonMark does - An inline target keeps balanced parentheses (guide(v2).md) and an angle-bracket target keeps its spaces (). - A reference definition may have its destination on the line after its label. Definitions are matched over the body, and a fence leaves a blank line behind. - A fenced block closes only at the same marker, at least as long as the opener. - A backtick info string with a backtick is an inline code span, not a fence. - A skill named through a subdirectory (skills/alpha/sub/..) takes the name of the directory it resolves to lexically, both for name checks and mirror checks. Tests: tests/test_catalog_links.py (new; test_catalog_validate.py is at the length cap). Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 53 ++++++++++--- dokimi/tests/test_catalog_links.py | 123 +++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 12 deletions(-) create mode 100644 dokimi/tests/test_catalog_links.py diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 059990427..4bc451fdc 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -54,6 +54,7 @@ from __future__ import annotations import hashlib +import os import re from dataclasses import dataclass, field from pathlib import Path, PurePosixPath @@ -87,12 +88,19 @@ should must may might if else only other such via per about over under more most very just""".split() ) -LINK_RE = re.compile(r"\]\(([^)\s]+)(?:\s+\"[^\"]*\")?\)") -# A reference-style link definition, ``[label]: destination`` (footnotes ``[^n]:`` excluded). +# An inline link target: balanced parentheses (``guide(v2).md``) or ```` with spaces; +# an optional title follows the target. +_TARGET_TITLE = r"""(?:\s+(?:"[^"]*"|'[^']*'|\([^()]*\)))?""" +LINK_RE = re.compile(rf"\]\(\s*(<[^<>\n]*>|(?:[^()\s]|\([^()\s]*\))+){_TARGET_TITLE}\s*\)") +# A reference-style link definition, ``[label]: destination``, the destination on this line or the +# next (footnotes ``[^n]:`` excluded). Matched over the whole body, not line by line. REF_DEF_RE = re.compile( - r"""^ {0,3}\[(?!\^)[^\]]+\]:[ \t]*(<[^>\n]*>|\S+)""" - r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""" + r"""^ {0,3}\[(?!\^)[^\]\n]+\]:[ \t]*\n?[ \t]*(<[^>\n]*>|\S+)""" + r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""", + re.MULTILINE, ) +FENCE_OPEN_RE = re.compile(r"[ \t]*(`{3,}|~{3,})(.*)$") +FENCE_CLOSE_RE = re.compile(r"[ \t]*(`{3,}|~{3,})[ \t]*$") # RFC 3986 scheme, dots included; a dotted one with a line or symbol after it is a file (_file_ref). URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-.]*:") # After a file, a line or a symbol (``run.py:42``, ``run.py:10-20``, ``run.py:main``): read as the file's name. @@ -150,7 +158,7 @@ def _frontmatter_findings(skill: Skill, *, at_root: bool = False) -> tuple[list[ meta, parse_error = _strict_meta(skill.path.read_text(encoding="utf-8")) if parse_error is not None: return [parse_error], False - dir_name = skill.path.parent.name + dir_name = _dir_name_of(skill.path.parent) findings: list[Finding] = [] declared = str(meta.get("name") or "").strip() if not declared: @@ -235,13 +243,26 @@ def _relative_to_root(path: Path, root: Path) -> str: def _outside_fences(body: str) -> list[str]: - """The lines of ``body`` that are not inside a fenced code block.""" + """The lines of ``body`` that are not inside a fenced code block. + + A block closes only at a line of the same marker (backticks or tildes) that is at least as long + as the opening run, as in CommonMark. + """ lines: list[str] = [] - fenced = False + fence = "" # the run that opened the current block; "" outside one for line in body.splitlines(): - if line.lstrip().startswith(("```", "~~~")): - fenced = not fenced - elif not fenced: + if fence: + close = FENCE_CLOSE_RE.match(line) + if close and close.group(1)[0] == fence[0] and len(close.group(1)) >= len(fence): + fence = "" + lines.append("") # a blank line keeps a definition above the block from reaching below it + continue + opened = FENCE_OPEN_RE.match(line) + # a backtick info string may not hold a backtick: "```x```" is an inline code span, not a fence + if opened and not (opened.group(1)[0] == "`" and "`" in opened.group(2)): + fence = opened.group(1) + lines.append("") + else: lines.append(line) return lines @@ -297,7 +318,7 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: lines = _outside_fences(skill.body) links = dict.fromkeys( [m for line in lines for m in LINK_RE.findall(line)] - + [m for line in lines for m in REF_DEF_RE.findall(line)] + + REF_DEF_RE.findall("\n".join(lines)) ) spans = dict.fromkeys( s.strip() @@ -373,6 +394,14 @@ def _jaccard(a: frozenset[str], b: frozenset[str]) -> float: return len(a & b) / len(a | b) if a and b else 0.0 +def _dir_name_of(directory: Path) -> str: + """The name of ``directory`` with its ``..`` parts taken away lexically: ``alpha/sub/..`` is ``alpha``. + + Lexical, not resolved, so a symlinked skill directory keeps the name it is listed under. + """ + return Path(os.path.normpath(directory)).name + + def _dir_name(item: tuple[SkillCatalogEntry, Skill, SkillReview]) -> Optional[str]: """The directory name that can make a skill a mirror of another, None for the root skill. @@ -382,7 +411,7 @@ def _dir_name(item: tuple[SkillCatalogEntry, Skill, SkillReview]) -> Optional[st entry, skill, _ = item if skill.path.parent.resolve() == entry.source_root.resolve(): return None - return skill.path.parent.name + return _dir_name_of(skill.path.parent) def _mirror_drift(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) -> None: diff --git a/dokimi/tests/test_catalog_links.py b/dokimi/tests/test_catalog_links.py new file mode 100644 index 000000000..bbecfc11b --- /dev/null +++ b/dokimi/tests/test_catalog_links.py @@ -0,0 +1,123 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Link targets, reference definitions, fenced blocks and directory names in ``dokimi.external.validate``. + +Split from ``test_catalog_validate.py``, which is over the file-length cap. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from dokimi.external import read_skill_manifest, review_skills + + +def _skill(root: Path, rel: str, name: str | None = None, description: str = "Use for things.", + extra: str = "", body: str = "# Body\n") -> Path: + """Write ``//SKILL.md``; ``name`` None leaves the key out of the frontmatter.""" + skill_dir = root / rel + skill_dir.mkdir(parents=True, exist_ok=True) + head = f"name: {name}\n" if name is not None else "" + (skill_dir / "SKILL.md").write_text( + f"---\n{head}description: {description}\n{extra}---\n{body}", encoding="utf-8") + return skill_dir + + +def _reviews_by_name(root: Path) -> dict: + entries = read_skill_manifest(root) + return {e.name: r for e, r in zip(entries, review_skills(entries), strict=True)} + + +def _codes(review) -> list[str]: + return [f.code for f in review.findings] + + +def test_a_link_destination_with_parentheses_to_an_existing_file_is_not_missing(tmp_path): + skill_dir = _skill(tmp_path / "skills", "parens", name="parens", description="Use for parens.", + body="See [the guide](references/guide(v2).md) and [a](references/a(b)(c).md).\n") + (skill_dir / "references").mkdir() + (skill_dir / "references" / "guide(v2).md").write_text("# Guide\n", encoding="utf-8") + (skill_dir / "references" / "a(b)(c).md").write_text("# A\n", encoding="utf-8") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +def test_a_link_destination_with_parentheses_to_a_missing_file_names_the_whole_path(tmp_path): + _skill(tmp_path / "skills", "parens-missing", name="parens-missing", description="Use for parens.", + body="See [the guide](references/guide(v2).md).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-missing"] + assert "'references/guide(v2).md'" in review.findings[0].message + + +@pytest.mark.parametrize("exists", [True, False]) +def test_angle_bracket_destination_with_spaces_is_checked(tmp_path, exists): + (tmp_path / "shared").mkdir() + if exists: + (tmp_path / "shared" / "my notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "spaced-link", name="spaced-link", description="Use for spaced link.", + body="See [notes](<../../shared/my notes.md>).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == (["link-escapes-skill"] if exists else ["link-missing"]) + + +@pytest.mark.parametrize("exists", [True, False]) +def test_reference_definition_with_destination_on_the_next_line_is_checked(tmp_path, exists): + (tmp_path / "shared").mkdir() + if exists: + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "next-line", name="next-line", description="Use for next line.", + body="See [notes][n].\n\n[n]:\n ../../shared/notes.md\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == (["link-escapes-skill"] if exists else ["link-missing"]) + + +def test_a_tilde_line_inside_a_backtick_fence_does_not_close_it(tmp_path): + _skill(tmp_path / "skills", "fences", name="fences", description="Use for fences.", + body="```markdown\n~~~\n[example](missing.md)\n```\n[real](also-missing.md)\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-missing"] + assert "'also-missing.md'" in review.findings[0].message + + +def test_a_closing_fence_shorter_than_the_opening_one_does_not_close_it(tmp_path): + _skill(tmp_path / "skills", "fence-run", name="fence-run", description="Use for fence run.", + body="````\n```\n[example](missing.md)\n````\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +def test_skills_named_through_a_subdirectory_dot_dot_are_not_mirror_drift(tmp_path): + # "skills/alpha/sub/.." is the skill directory "alpha": two such skills are not copies of each other. + _skill(tmp_path / "skills", "alpha", name="alpha", description="Use for alpha work.", body="# Alpha\n") + _skill(tmp_path / "skills", "beta", name="beta", description="Use for beta work.", body="# Beta\n") + (tmp_path / "skills" / "alpha" / "sub").mkdir() + (tmp_path / "skills" / "beta" / "sub").mkdir() + (tmp_path / "skills.yaml").write_text( + "skills:\n" + " - name: alpha\n catalog_path: skills/alpha/sub/..\n" + " - name: beta\n catalog_path: skills/beta/sub/..\n", + encoding="utf-8") + + reviews = _reviews_by_name(tmp_path) + assert _codes(reviews["alpha"]) == [] + assert _codes(reviews["beta"]) == [] + + +def test_a_reference_label_before_a_fence_does_not_take_a_destination_after_it(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") + _skill(tmp_path / "skills", "label-fence", name="label-fence", description="Use for label fence.", + body="See [notes][n].\n\n[n]:\n```\ncode\n```\n../../shared/notes.md\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] From e6a18b34bf4353d01ab803b179ad0acccd95424d Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 22:07:28 +0000 Subject: [PATCH 16/22] Make the separator in REF_DEF_RE unambiguous Signed-off-by: Anton Volkov --- dokimi/dokimi/external/validate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 4bc451fdc..73d9f33a8 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -95,7 +95,7 @@ # A reference-style link definition, ``[label]: destination``, the destination on this line or the # next (footnotes ``[^n]:`` excluded). Matched over the whole body, not line by line. REF_DEF_RE = re.compile( - r"""^ {0,3}\[(?!\^)[^\]\n]+\]:[ \t]*\n?[ \t]*(<[^>\n]*>|\S+)""" + r"""^ {0,3}\[(?!\^)[^\]\n]+\]:[ \t]*(?:\n[ \t]*)?(<[^>\n]*>|\S+)""" r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""", re.MULTILINE, ) From 48c83079f199c571721cc015eed9114c2c223843 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 22:07:40 +0000 Subject: [PATCH 17/22] Cap the indent of a code fence at three spaces Signed-off-by: Anton Volkov --- dokimi/dokimi/external/validate.py | 4 ++-- dokimi/tests/test_catalog_links.py | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 73d9f33a8..285db5076 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -99,8 +99,8 @@ r"""(?:[ \t]+(?:"[^"]*"|'[^']*'|\([^)]*\)))?[ \t]*$""", re.MULTILINE, ) -FENCE_OPEN_RE = re.compile(r"[ \t]*(`{3,}|~{3,})(.*)$") -FENCE_CLOSE_RE = re.compile(r"[ \t]*(`{3,}|~{3,})[ \t]*$") +FENCE_OPEN_RE = re.compile(r"\ {0,3}(`{3,}|~{3,})(.*)$") +FENCE_CLOSE_RE = re.compile(r"\ {0,3}(`{3,}|~{3,})[ \t]*$") # RFC 3986 scheme, dots included; a dotted one with a line or symbol after it is a file (_file_ref). URI_SCHEME_RE = re.compile(r"[A-Za-z][A-Za-z0-9+\-.]*:") # After a file, a line or a symbol (``run.py:42``, ``run.py:10-20``, ``run.py:main``): read as the file's name. diff --git a/dokimi/tests/test_catalog_links.py b/dokimi/tests/test_catalog_links.py index bbecfc11b..ed411ea48 100644 --- a/dokimi/tests/test_catalog_links.py +++ b/dokimi/tests/test_catalog_links.py @@ -96,6 +96,14 @@ def test_a_closing_fence_shorter_than_the_opening_one_does_not_close_it(tmp_path assert review.findings == [] +def test_a_fence_indented_four_spaces_does_not_open_a_block(tmp_path): + _skill(tmp_path / "skills", "indented", name="indented", description="Use for indented.", + body=" ```\n[a](gone.md)\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-missing"] + + def test_skills_named_through_a_subdirectory_dot_dot_are_not_mirror_drift(tmp_path): # "skills/alpha/sub/.." is the skill directory "alpha": two such skills are not copies of each other. _skill(tmp_path / "skills", "alpha", name="alpha", description="Use for alpha work.", body="# Alpha\n") From 21c47670b5f4305de9fcfec00df69e5bcd20bdcb Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Sun, 1 Nov 2026 22:07:52 +0000 Subject: [PATCH 18/22] Test link titles, padded targets and fence lookalikes Signed-off-by: Anton Volkov --- dokimi/tests/test_catalog_links.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dokimi/tests/test_catalog_links.py b/dokimi/tests/test_catalog_links.py index ed411ea48..0291011b0 100644 --- a/dokimi/tests/test_catalog_links.py +++ b/dokimi/tests/test_catalog_links.py @@ -121,6 +121,29 @@ def test_skills_named_through_a_subdirectory_dot_dot_are_not_mirror_drift(tmp_pa assert _codes(reviews["beta"]) == [] +@pytest.mark.parametrize("body", [ + '[a](gone.md "t")\n', + "[a](gone.md 't')\n", + "[a](gone.md (t))\n", + "[a]( gone.md )\n", + "```x``` then\n[a](gone.md)\n", +]) +def test_link_forms_and_fence_lookalikes_are_still_checked(tmp_path, body): + _skill(tmp_path / "skills", "forms", name="forms", description="Use for forms.", body=body) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-missing"] + + +def test_a_link_inside_parentheses_does_not_take_the_closing_one(tmp_path): + skill_dir = _skill(tmp_path / "skills", "wrapped", name="wrapped", description="Use for wrapped.", + body="(see [a](here.md))\n") + (skill_dir / "here.md").write_text("# Here\n", encoding="utf-8") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + def test_a_reference_label_before_a_fence_does_not_take_a_destination_after_it(tmp_path): (tmp_path / "shared").mkdir() (tmp_path / "shared" / "notes.md").write_text("# Notes\n", encoding="utf-8") From 625632b1f74ee595887b757ce0ee88112913fb91 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 16:53:33 -0800 Subject: [PATCH 19/22] fix(catalog): trigger phrases and link queries match whole words and paths A use phrase now needs its last word to end (use for, use if), so 'Use formatting tools' no longer passes as a when clause. A link destination's ?query is not part of the file it names. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 15 +++++++++------ dokimi/tests/test_catalog_links.py | 20 ++++++++++++++++++++ dokimi/tests/test_catalog_validate.py | 15 +++++++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 285db5076..e03ad5580 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -75,9 +75,9 @@ # A "when" clause of any subject and verb ("run migrations when deploying"), or a use phrase: the model # chooses a skill from its description alone. WHEN_RE = re.compile( - r"\b(?:when\s+\w|use for|use this skill|trigger(?:s|ed)?\s*(?:on|:)|invoke only|activate\b|" - r"invoke this|use (?:it |this |these )?(?:when|if|before|after|whenever|proactively|anytime)|" - r"whenever)", + r"\b(?:when\s+\w|use for\b|use this skill\b|trigger(?:s|ed)?\s*(?:on\b|:)|invoke only\b|activate\b|" + r"invoke this\b|use (?:it |this |these )?(?:when|if|before|after|whenever|proactively|anytime)\b|" + r"whenever\b)", re.I, ) # Words that say nothing about what a skill is for; left out of the description overlap. @@ -267,17 +267,20 @@ def _outside_fences(body: str) -> list[str]: return lines -def _file_ref(raw: str) -> Optional[str]: +def _file_ref(raw: str, link: bool = False) -> Optional[str]: """The skill-relative file a reference names, or None if it names no file to check. None for a URL or any other URI (``tel:``, ``urn:``, ``MAILTO:``), a bare anchor, an absolute path, or a template with a variable other than ``${CLAUDE_SKILL_DIR}``. A - ``<...>`` destination (CommonMark) is read as the path inside the brackets. + ``<...>`` destination (CommonMark) is read as the path inside the brackets. A link + destination (``link``) is a URL: its ``?query`` is not part of the path. """ text = raw.strip() if text.startswith("<") and text.endswith(">"): text = text[1:-1].strip() text = text.split("#", 1)[0].strip() + if link: + text = text.split("?", 1)[0].strip() if not text or "://" in text or text.startswith("/"): return None scheme = URI_SCHEME_RE.match(text) @@ -328,7 +331,7 @@ def _bundle_findings(skill: Skill, source_root: Path) -> list[Finding]: ) findings: list[Finding] = [] for raw in links: - text = _file_ref(raw) + text = _file_ref(raw, link=True) if text is None: continue target = _resolved(skill_dir / text) diff --git a/dokimi/tests/test_catalog_links.py b/dokimi/tests/test_catalog_links.py index 0291011b0..9472f1822 100644 --- a/dokimi/tests/test_catalog_links.py +++ b/dokimi/tests/test_catalog_links.py @@ -152,3 +152,23 @@ def test_a_reference_label_before_a_fence_does_not_take_a_destination_after_it(t [review] = review_skills(read_skill_manifest(tmp_path)) assert review.findings == [] + + +def test_a_link_with_a_query_to_an_existing_file_is_checked_by_its_path(tmp_path): + skill_dir = _skill(tmp_path / "skills", "query-link", name="query-link", description="Use for query links.", + body="See [the guide](references/guide.md?raw=1).\n") + (skill_dir / "references").mkdir() + (skill_dir / "references" / "guide.md").write_text("# Guide\n", encoding="utf-8") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +def test_a_query_bearing_link_outside_the_skill_still_escapes(tmp_path): + (tmp_path / "shared").mkdir() + (tmp_path / "shared" / "guide.md").write_text("# Guide\n", encoding="utf-8") + _skill(tmp_path / "skills", "query-escape", name="query-escape", description="Use for query escapes.", + body="See [the guide](../../shared/guide.md?raw=1#top).\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["link-escapes-skill"] diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 11dca3079..70d53c815 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -957,3 +957,18 @@ def test_an_escaping_link_with_a_line_suffix_is_named_as_escaping(tmp_path): [review] = review_skills(read_skill_manifest(tmp_path)) assert sorted(_codes(review)) == ["link-escapes-skill", "path-escapes-skill"] + + +@pytest.mark.parametrize( + "description", + [ + "Use formatting tools to normalize logs.", + "Use iframe helpers to inspect embedded pages.", + "Use ifs-free helpers to rewrite conditions.", + ], +) +def test_a_use_phrase_that_only_starts_like_a_trigger_is_still_a_description_warning(tmp_path, description): + _skill(tmp_path / "skills", "prefix-only", name="prefix-only", description=description) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert _codes(review) == ["description-no-when"] From 219e4cf0dc79ff44b23c1799b2335dd65b00e00e Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 1 Nov 2026 20:29:58 -0800 Subject: [PATCH 20/22] fix(catalog): name the other checkout in mirror-drift and near-duplicate findings Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 25 +++++++++++++++++-------- dokimi/tests/test_catalog_validate.py | 14 ++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index e03ad5580..ab0242d24 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -242,6 +242,15 @@ def _relative_to_root(path: Path, root: Path) -> str: return path.as_posix() +def _counterpart(other: tuple[SkillCatalogEntry, Skill, SkillReview], entry: SkillCatalogEntry) -> str: + """The other skill's directory relative to its root, qualified by that root when it is another checkout.""" + o_entry, o_skill, _ = other + where = _relative_to_root(o_skill.path.parent, o_entry.source_root) + if o_entry.source_root.resolve() == entry.source_root.resolve(): + return where + return f"{where} in {o_entry.source_root.as_posix()}" + + def _outside_fences(body: str) -> list[str]: """The lines of ``body`` that are not inside a fenced code block. @@ -425,11 +434,11 @@ def _mirror_drift(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) -> if name is not None: by_dir_name.setdefault(name, []).append(item) for group in by_dir_name.values(): - for _, _, review in group: + for entry, _, review in group: others = [ - _relative_to_root(o_skill.path.parent, o_entry.source_root) - for o_entry, o_skill, o_review in group - if o_review.content_sha256 != review.content_sha256 + _counterpart(item, entry) + for item in group + if item[2].content_sha256 != review.content_sha256 ] if others: review.findings.append( @@ -451,10 +460,10 @@ def _near_duplicates(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) for index, words in enumerate(tokens): for word in words: by_token.setdefault(word, []).append(index) - for i, (entry, skill, review) in enumerate(shown): + for i, (entry, _, review) in enumerate(shown): # only a pair that shares a word can overlap for j in sorted({k for word in tokens[i] for k in by_token[word] if k > i}): - o_entry, o_skill, o_review = shown[j] + o_entry, _, o_review = shown[j] if o_review.content_sha256 == review.content_sha256: continue # an identical SKILL.md: already a duplicate if names[i] is not None and names[i] == names[j]: @@ -462,8 +471,8 @@ def _near_duplicates(loaded: list[tuple[SkillCatalogEntry, Skill, SkillReview]]) similarity = _jaccard(tokens[i], tokens[j]) if similarity >= NEAR_DUPLICATE_JACCARD: pairs = ( - (review, _relative_to_root(o_skill.path.parent, o_entry.source_root)), - (o_review, _relative_to_root(skill.path.parent, entry.source_root)), + (review, _counterpart(shown[j], entry)), + (o_review, _counterpart(shown[i], o_entry)), ) for side, other in pairs: side.findings.append( diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 70d53c815..cce832f19 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -896,6 +896,20 @@ def test_root_skills_of_two_checkouts_are_near_duplicates_not_mirror_drift(tmp_p reviews = review_skills(entries) assert [_codes(r) for r in reviews] == [["description-near-duplicate"]] * 2 + for review, other in zip(reviews, ("b/repo", "a/repo"), strict=True): + assert str((tmp_path / other).as_posix()) in review.findings[0].message + + +def test_mirror_drift_names_the_checkout_of_the_other_copy(tmp_path): + entries = [] + for checkout, text in (("a", "one"), ("b", "two")): + root = tmp_path / checkout + _skill(root, "same", name="same", description=f"Use for {text}.") + entries.extend(read_skill_manifest(root)) + + reviews = review_skills(entries) + for review, other in zip(reviews, ("b", "a"), strict=True): + assert (tmp_path / other).as_posix() in review.findings[0].message def test_near_duplicate_check_reads_non_ascii_description_words(tmp_path): From c46fd31a999cbada9252750241e9b374076c41ad Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 11:17:41 +0000 Subject: [PATCH 21/22] Test the word boundaries of the remaining WHEN_RE phrases Signed-off-by: Anton Volkov --- dokimi/tests/test_catalog_validate.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index cce832f19..de3270af6 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -979,6 +979,9 @@ def test_an_escaping_link_with_a_line_suffix_is_named_as_escaping(tmp_path): "Use formatting tools to normalize logs.", "Use iframe helpers to inspect embedded pages.", "Use ifs-free helpers to rewrite conditions.", + "Use this skillset to lint sources.", + "Triggers online for logs.", + "Wheneverish tooling for logs.", ], ) def test_a_use_phrase_that_only_starts_like_a_trigger_is_still_a_description_warning(tmp_path, description): From f1968ff3deb6f64c92fd761977c184f7e3dd83b2 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 2 Nov 2026 11:17:49 +0000 Subject: [PATCH 22/22] Pin the same-checkout form of the mirror-drift counterpart Signed-off-by: Anton Volkov --- dokimi/tests/test_catalog_validate.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index de3270af6..727a0bdeb 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -679,8 +679,8 @@ def test_drifted_copies_of_one_skill_warn_on_both_and_name_each_other(tmp_path): reviews = review_skills(read_skill_manifest(tmp_path)) assert all(r.duplicate_of is None for r in reviews) assert [_codes(r) for r in reviews] == [["mirror-drift"], ["mirror-drift"]] - assert "differs from plugins/x" in reviews[1].findings[0].message - assert "differs from skills/x" in reviews[0].findings[0].message + assert "differs from plugins/x (" in reviews[1].findings[0].message + assert "differs from skills/x (" in reviews[0].findings[0].message def test_identical_copies_are_a_duplicate_and_not_drift(tmp_path):