stack: SKILL.md-Links auf repo-root-relative Pfade umgestellt, docs verify/instructions verify pruefen Linkziele
Files changed: - CHANGES.md - VERSION - instructions/CONTRACT.md - instructions/dev/doc-pull-through.md - instructions/dev/stack-close/SKILL.md - instructions/dev/stack-dev/SKILL.md - instructions/wiki-ingest/SKILL.md - instructions/wiki-lint/SKILL.md - instructions/wiki-manage/SKILL.md - instructions/wiki-query/SKILL.md - instructions/wiki-status/SKILL.md - tools/CONTRACT.md - tools/chemenu/commands/docs_verify.py - tools/chemenu/commands/instructions_cmd.py - tools/chemenu/tests/test_docs_verify.py - tools/chemenu/tests/test_instructions_cmd.py
This commit is contained in:
+4
-4
@@ -168,9 +168,9 @@ tools/wikitool <command> --help
|
||||
| `types list [--json]` | List every type-spec under `types/` (name, schema path, subtype field, description) - discover what page types exist without reading `types/*.md` directly |
|
||||
| `types describe <name> [--json]` | Print one type's full contract: required/optional frontmatter fields with enums, its subtype field (if any), and its authoring body |
|
||||
| `instructions sync [--force]` | Publish every `instructions/<name>/SKILL.md` into `.agents/skills/` and `.claude/skills/` as **copies**, and delete published skills whose source is gone. Both targets are gitignored, so a fresh clone runs this once - see `instructions/bootstrap.md`. Re-running is also how a drifted copy is repaired: the source always wins. `--force` is required only to replace a target directory that is not a published skill at all (no `SKILL.md` in it) |
|
||||
| `instructions verify` | Check the instruction layer: flat instructions validate against `types/instruction.schema.yaml`, each `SKILL.md` carries the frontmatter its harness reads, every published copy is byte-identical to its source, no instruction is left that nothing references, and nothing under `instructions/dev/` is referenced from outside it (a `<!-- dist:strip-start/end -->` block is exempt - see [instructions/CONTRACT.md](../instructions/CONTRACT.md)). Missing *every* copy is reported as "run sync", not as drift - that is a clean checkout |
|
||||
| `instructions verify` | Check the instruction layer: flat instructions validate against `types/instruction.schema.yaml`, each `SKILL.md` carries the frontmatter its harness reads, no `SKILL.md` carries a relative markdown link (`sync` copies it to a different depth than the source, so a `SKILL.md` references a target as a repo-root-relative plain path instead - see [instructions/CONTRACT.md](../instructions/CONTRACT.md) § "A skill's outbound reference is a plain path, not a link"), every published copy is byte-identical to its source, no instruction is left that nothing references, and nothing under `instructions/dev/` is referenced from outside it (a `<!-- dist:strip-start/end -->` block is exempt - see [instructions/CONTRACT.md](../instructions/CONTRACT.md)). Missing *every* copy is reported as "run sync", not as drift - that is a clean checkout |
|
||||
| `instructions list [--json]` | List the flat instructions with their descriptions. This is how the layer is discovered; `search` deliberately covers `kb/` only |
|
||||
| `docs verify` | Check the docs that mirror the code: every CLI command documented in this file's own § Commands table and, separately, in its § Error contracts table (both directions, checked per table, so a row dropped from one is not hidden by the same name surviving in the other, and only a name's presence in a row is checked, never the rest of that row's text), every directory under `kb/` has a `COLLECTION.md` and no directory outside it does, every collection declaring `profile:` and a `required_by_stack:` that agrees with the stack's own list, `kb/CONVENTIONS.md` naming all three tool-owned section headings if it exists at all, every stage contract present, no pre-migration `type: entity` blocks left in the contracts, the `.gitignore` canaries clear in both directions (nothing ignored under `raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the published skill directories), and no `.md`/`.template` file `dist export` would ship citing an issue number - the tracker exists only in the origin repo, so such a number in a distributed instance is a reference its reader can neither resolve nor recognise as unresolvable (a `<!-- dist:strip-start/end -->` region is exempt: it is already gone from the text the check reads, which is the export plan's, not the working tree's), and every reference file `docs toc` covers carrying the current table-of-contents region for its own headings - missing and stale are one check, because the generator is idempotent. The name is about documentation parity, not about the `docs/` directory - it neither reads nor requires one, the same way `kb/` predates the collection it now checks |
|
||||
| `docs verify` | Check the docs that mirror the code: every CLI command documented in this file's own § Commands table and, separately, in its § Error contracts table (both directions, checked per table, so a row dropped from one is not hidden by the same name surviving in the other, and only a name's presence in a row is checked, never the rest of that row's text), every directory under `kb/` has a `COLLECTION.md` and no directory outside it does, every collection declaring `profile:` and a `required_by_stack:` that agrees with the stack's own list, `kb/CONVENTIONS.md` naming all three tool-owned section headings if it exists at all, every stage contract present, no pre-migration `type: entity` blocks left in the contracts, the `.gitignore` canaries clear in both directions (nothing ignored under `raw/`/`kb/`, `incoming/` ignored, everything ignored under `reports/` and the published skill directories), and no `.md`/`.template` file `dist export` would ship citing an issue number - the tracker exists only in the origin repo, so such a number in a distributed instance is a reference its reader can neither resolve nor recognise as unresolvable (a `<!-- dist:strip-start/end -->` region is exempt: it is already gone from the text the check reads, which is the export plan's, not the working tree's), every reference file `docs toc` covers carrying the current table-of-contents region for its own headings - missing and stale are one check, because the generator is idempotent - and every relative markdown link in one of those same reference files resolving to a file that actually exists (a target's `#anchor` suffix is stripped first; code fences and inline code spans are masked before scanning, so a passage showing link syntax as an example is not mistaken for a real reference). The name is about documentation parity, not about the `docs/` directory - it neither reads nor requires one, the same way `kb/` predates the collection it now checks |
|
||||
| `docs toc [--apply]` | Create, refresh or remove the generated table-of-contents region (`<!-- wikitool:toc -->` ... `<!-- /wikitool:toc -->`, placed after the title and before the first `##`) on every reference file over 100 lines that `AGENTS.md`, a stage/collection contract, or the flat `instructions/**.md` form covers - the scope Anthropic's skill-authoring guidance names for a file previewed rather than read in full. Dry-run by default (prints which files would change); `--apply` writes. `docs verify` checks the result stays current the same way it checks every other generated-from-code copy |
|
||||
|
||||
### Telemetry
|
||||
@@ -379,9 +379,9 @@ is atomic, and whether a retry is safe.
|
||||
| `types list` | Never fails | Read-only | Safe to retry freely |
|
||||
| `types describe` | Unknown type name | Read-only | Fix the name and retry |
|
||||
| `instructions sync` | No skills found under `instructions/`, or a target directory is not a published skill (no `SKILL.md`) and `--force` was not passed | No - one directory copy per skill per target (`.agents/skills/`, `.claude/skills/`); each copy is idempotent, so a re-run converges even after a partial failure | Check whether the flagged target holds anything worth keeping, then re-run with `--force` if not; otherwise fix the named cause and retry |
|
||||
| `instructions verify` | Nothing found under `instructions/` at all, a malformed instruction or `SKILL.md`, a published copy that drifted from its source, an instruction nothing references (or, for `manual: true`, one that IS linked from AGENTS.md, CLAUDE.md, or a skill and so risks running implicitly), or something under `instructions/dev/` referenced from outside it and outside a `dist:strip` block | Read-only | Fix the flagged file, then re-run. For drift, re-run `sync` instead of hand-editing the published copy - the source under `instructions/` always wins |
|
||||
| `instructions verify` | Nothing found under `instructions/` at all, a malformed instruction or `SKILL.md`, a `SKILL.md` carrying a relative markdown link, a published copy that drifted from its source, an instruction nothing references (or, for `manual: true`, one that IS linked from AGENTS.md, CLAUDE.md, or a skill and so risks running implicitly), or something under `instructions/dev/` referenced from outside it and outside a `dist:strip` block | Read-only | Fix the flagged file, then re-run. For a relative link in a `SKILL.md`, rewrite it as a repo-root-relative plain path instead. For drift, re-run `sync` instead of hand-editing the published copy - the source under `instructions/` always wins |
|
||||
| `instructions list` | Never fails - an empty `instructions/` prints "No instructions found." | Read-only | Safe to retry freely |
|
||||
| `docs verify` | A command, contract, or type-form mismatch was found, a shipped `.md`/`.template` cites an issue number, or a reference file's table-of-contents region is missing or stale | Read-only | Fix the documentation it names, then re-run. For an issue reference: say what was decided instead of pointing at where, or move the pointer behind a `<!-- dist:strip-start/end -->` block. For a table of contents: run `docs toc --apply` - never hand-write the region |
|
||||
| `docs verify` | A command, contract, or type-form mismatch was found, a shipped `.md`/`.template` cites an issue number, a reference file's table-of-contents region is missing or stale, or a reference file's relative markdown link does not resolve to an existing file | Read-only | Fix the documentation it names, then re-run. For an issue reference: say what was decided instead of pointing at where, or move the pointer behind a `<!-- dist:strip-start/end -->` block. For a table of contents: run `docs toc --apply` - never hand-write the region. For a dead link: fix the `../` count or the target's name |
|
||||
| `docs toc` | Never fails on content: a file with no `##` heading, or one at or under the threshold, is simply left without a region | `--apply` rewrites each named file in place, one at a time and idempotently, so a re-run after an interruption converges rather than doubling a region; the dry-run form is read-only | Nothing to fix - re-run with `--apply` to write what the dry run listed. If `docs verify` still reports a stale region afterwards, the file's `##` headings changed in between; run it again |
|
||||
|
||||
### Telemetry
|
||||
|
||||
@@ -32,6 +32,18 @@ A sixth checks a *reference* rather than a copy: no document `dist export`
|
||||
ships may cite an issue number, because the board those numbers live on
|
||||
exists only in the origin repo.
|
||||
|
||||
A seventh checks the other half of the same reference problem: every relative
|
||||
markdown link in a reference file - `toc.target_files()`'s scope, the same one
|
||||
the table-of-contents check uses - must resolve to a file that actually
|
||||
exists. A link with the wrong `../` count is invisible to every check above:
|
||||
it is present, it names an existing command or contract by title, and nothing
|
||||
renders it to notice the target is unreachable. The complementary half - that
|
||||
`instructions/<name>/SKILL.md` never carries a relative markdown link at all,
|
||||
because `instructions sync` copies it to a different depth than its links
|
||||
assume - is `instructions verify`'s job, not this one, since that module
|
||||
already owns the Skill/Instruction split (`skill_dirs()` vs
|
||||
`instruction_files()`).
|
||||
|
||||
Everything here is a hard oracle: a set comparison or a regex, no judgment.
|
||||
Content quality of the contracts themselves stays with the LLM.
|
||||
"""
|
||||
@@ -44,7 +56,7 @@ from typing import Optional
|
||||
|
||||
import typer
|
||||
|
||||
from chemenu import config, conventions, kb_collections, toc, version as version_mod
|
||||
from chemenu import config, conventions, kb_collections, markdown_code, toc, version as version_mod
|
||||
from chemenu.commands import dist_cmd
|
||||
from chemenu.commands._util import fail, rel_path, success
|
||||
|
||||
@@ -452,6 +464,61 @@ def check_toc_regions() -> list[str]:
|
||||
return issues
|
||||
|
||||
|
||||
# A markdown link, `[text](target)`. The target excludes `)` and whitespace -
|
||||
# the same restriction every link in this repo's own instructions already
|
||||
# follows; a target needing either would need CommonMark's <angle-bracket>
|
||||
# escaping, which nothing here uses.
|
||||
MARKDOWN_LINK_RE = re.compile(r"\[[^\]]*\]\(([^)\s]+)\)")
|
||||
|
||||
|
||||
def is_external_or_anchor(target: str) -> bool:
|
||||
"""A link this check does not resolve as a filesystem path: an absolute
|
||||
URL, a `mailto:`, or a pure in-page `#anchor`.
|
||||
|
||||
Public (not `_`-prefixed): `instructions_cmd.check_skill_reference_paths`
|
||||
imports this alongside `MARKDOWN_LINK_RE` rather than keeping a second
|
||||
copy - the two checks classify the same link shape, just over different
|
||||
file sets (AGENTS.md invariant 8)."""
|
||||
return target.startswith(("http://", "https://", "mailto:", "#"))
|
||||
|
||||
|
||||
def check_reference_targets() -> list[str]:
|
||||
"""Every relative markdown link in a reference file resolves to a real file.
|
||||
|
||||
Scoped to `toc.target_files()` - AGENTS.md, the stage and collection
|
||||
contracts, and every flat `instructions/**.md` file - the same scope the
|
||||
table-of-contents check uses. That scope already excludes `SKILL.md`
|
||||
(banned from carrying a markdown link at all - `instructions verify`'s
|
||||
`check_skill_reference_paths`), `commonplace/` (vendored, not stack
|
||||
material) and `raw/`/`kb/` page content (data, not documentation) beyond
|
||||
the two files that are themselves reference material.
|
||||
|
||||
A target's `#anchor` suffix is stripped before resolving - CommonMark
|
||||
anchors are not filesystem paths, and nothing here renders one to notice
|
||||
a stale one anyway. Code fences and inline code spans are masked first
|
||||
(`markdown_code.strip_code_spans`), so a passage that shows link syntax
|
||||
as an example is not mistaken for a real reference.
|
||||
"""
|
||||
issues = []
|
||||
for path in toc.target_files():
|
||||
text = path.read_text(encoding="utf-8")
|
||||
masked = markdown_code.strip_code_spans(text)
|
||||
for line_number, masked_line in enumerate(masked.splitlines(), start=1):
|
||||
for match in MARKDOWN_LINK_RE.finditer(masked_line):
|
||||
target = match.group(1)
|
||||
if is_external_or_anchor(target):
|
||||
continue
|
||||
target_path = target.split("#", 1)[0]
|
||||
if not target_path:
|
||||
continue
|
||||
if not (path.parent / target_path).resolve().exists():
|
||||
issues.append(
|
||||
f"{rel_path(path)}:{line_number} links to `{target}`, which does not "
|
||||
"resolve to an existing file"
|
||||
)
|
||||
return issues
|
||||
|
||||
|
||||
def command_table_free_readmes() -> list[Path]:
|
||||
"""Every README that must not carry a copy of the command table.
|
||||
|
||||
@@ -768,7 +835,7 @@ def check_breaking_change_for_boundary() -> list[str]:
|
||||
|
||||
@app.command("verify")
|
||||
def verify():
|
||||
"""Check the CLI/README command tables, contract presence, type-form drift, ignore rules, version/changelog agreement, and issue references in shipped documents."""
|
||||
"""Check the CLI/README command tables, contract presence, type-form drift, ignore rules, version/changelog agreement, issue references, and link targets in shipped documents."""
|
||||
issues = (
|
||||
check_cli_readme()
|
||||
+ check_readmes_have_no_command_table()
|
||||
@@ -780,6 +847,7 @@ def verify():
|
||||
+ check_breaking_change_for_boundary()
|
||||
+ check_no_issue_references()
|
||||
+ check_toc_regions()
|
||||
+ check_reference_targets()
|
||||
)
|
||||
|
||||
if issues:
|
||||
@@ -791,7 +859,8 @@ def verify():
|
||||
f"{len(STAGE_CONTRACTS)} stage contract(s) present, no legacy type blocks, "
|
||||
f"{len(IGNORE_CANARIES)} ignore canaries clear, "
|
||||
f"no issue references in {len(shipped_prose())} shipped document(s), "
|
||||
f"tables of contents current on {len(toc.target_files())} reference file(s), "
|
||||
f"tables of contents current and every link resolving on "
|
||||
f"{len(toc.target_files())} reference file(s), "
|
||||
f"{version_mod.CHANGES_FILENAME} documents version "
|
||||
f"{(config.ROOT / version_mod.VERSION_FILENAME).read_text(encoding='utf-8').strip()}."
|
||||
)
|
||||
|
||||
@@ -20,6 +20,19 @@ Both target directories are gitignored. A fresh clone has no skills until `sync`
|
||||
runs; `instructions/bootstrap.md` is the procedure, and `verify` says so rather
|
||||
than reporting an error when *every* copy is missing, because that is the
|
||||
expected state of a clean checkout rather than a fault.
|
||||
|
||||
The copy is also a different depth than the source, and without the sibling
|
||||
files a relative link might expect - a plain `shutil.copytree` per skill
|
||||
directory, not a mirror of the whole `instructions/` tree. A relative markdown
|
||||
link correct at `instructions/<name>/SKILL.md` therefore resolves to a
|
||||
different, usually nonexistent, file in the published copy the harness
|
||||
actually reads. `verify` forbids the shape outright
|
||||
(`check_skill_reference_paths`) rather than checking depth arithmetic, and a
|
||||
`SKILL.md` writes an outbound reference as a repo-root-relative plain path
|
||||
instead - see instructions/CONTRACT.md § "A skill's outbound reference is a
|
||||
plain path, not a link". `docs_verify.check_reference_targets` is the
|
||||
complementary check, over the flat instructions and contracts that are still
|
||||
allowed to link normally because nothing ever copies them elsewhere.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -31,8 +44,8 @@ from pathlib import Path
|
||||
import typer
|
||||
import yaml
|
||||
|
||||
from chemenu import config
|
||||
from chemenu.commands import dist_cmd
|
||||
from chemenu import config, markdown_code
|
||||
from chemenu.commands import dist_cmd, docs_verify
|
||||
from chemenu.commands._util import fail, rel_path, success
|
||||
from chemenu.type_resolver import resolver
|
||||
|
||||
@@ -313,6 +326,45 @@ def dev_only_forbidden_references(instructions_dir: Path | None = None) -> set[s
|
||||
return referenced
|
||||
|
||||
|
||||
def check_skill_reference_paths() -> list[str]:
|
||||
"""No `SKILL.md` may carry a relative markdown link.
|
||||
|
||||
`sync` copies each skill directory verbatim into `.agents/skills/<name>/`
|
||||
and `.claude/skills/<name>/` - a different depth than
|
||||
`instructions/<name>/SKILL.md` itself, and without the sibling files a
|
||||
relative link might expect. A markdown link that resolves correctly at
|
||||
the source (`../session-setup.md`, `../../kb/CONTRACT.md`) resolves to a
|
||||
different, usually nonexistent, file once copied: the number of `../`
|
||||
segments that reaches a target from `instructions/<name>/` does not reach
|
||||
the same target from `.claude/skills/<name>/`.
|
||||
|
||||
So a `SKILL.md` never writes an outbound reference as a relative markdown
|
||||
link - it names the target as a repo-root-relative plain path instead
|
||||
(`` `instructions/session-setup.md` ``, not
|
||||
`[session-setup.md](../session-setup.md)`). See instructions/CONTRACT.md
|
||||
§ "A skill's outbound reference is a plain path, not a link" for why that
|
||||
form survives the copy unchanged.
|
||||
`docs_verify.check_reference_targets` is the complementary check, over the
|
||||
flat instructions and contracts that are still allowed to link normally
|
||||
because nothing ever copies them elsewhere."""
|
||||
issues: list[str] = []
|
||||
for source in skill_dirs():
|
||||
path = source / SKILL_FILE
|
||||
text = path.read_text(encoding="utf-8")
|
||||
masked = markdown_code.strip_code_spans(text)
|
||||
for line_number, masked_line in enumerate(masked.splitlines(), start=1):
|
||||
for match in docs_verify.MARKDOWN_LINK_RE.finditer(masked_line):
|
||||
target = match.group(1)
|
||||
if docs_verify.is_external_or_anchor(target):
|
||||
continue
|
||||
issues.append(
|
||||
f"{rel_path(path)}:{line_number} carries a relative markdown link to "
|
||||
f"`{target}` - `instructions sync` copies this file to a different depth, "
|
||||
"so write the target as a plain repo-root-relative path instead"
|
||||
)
|
||||
return issues
|
||||
|
||||
|
||||
@app.command("sync")
|
||||
def sync(
|
||||
force: bool = typer.Option(
|
||||
@@ -351,7 +403,7 @@ def sync(
|
||||
|
||||
@app.command("verify")
|
||||
def verify():
|
||||
"""Check instructions/ against its type, and every published copy against its source."""
|
||||
"""Check instructions/ against its type, that no skill carries a relative markdown link, and every published copy against its source."""
|
||||
sources = skill_dirs()
|
||||
instructions = instruction_files()
|
||||
if not sources and not instructions:
|
||||
@@ -398,7 +450,11 @@ def verify():
|
||||
if not frontmatter.get("description"):
|
||||
issues.append(f"{source.name}: SKILL.md is missing (or has an empty) `description`")
|
||||
|
||||
# 3. Published copies match their sources. Missing *everywhere* is a clean
|
||||
# 3. No skill carries a relative markdown link - see
|
||||
# check_skill_reference_paths's own docstring for why the copy breaks it.
|
||||
issues.extend(check_skill_reference_paths())
|
||||
|
||||
# 4. Published copies match their sources. Missing *everywhere* is a clean
|
||||
# checkout, not a fault - say what to run instead of reporting drift.
|
||||
expected = len(sources) * len(target_dirs())
|
||||
missing = 0
|
||||
@@ -419,7 +475,7 @@ def verify():
|
||||
if missing and not bootstrap_needed:
|
||||
issues.append(f"{missing} published copy/copies missing - run `wikitool instructions sync`")
|
||||
|
||||
# 4. An instruction nothing loads is inert. Nothing else would report it -
|
||||
# 5. An instruction nothing loads is inert. Nothing else would report it -
|
||||
# unless it is `manual: true`, which inverts the rule over a narrower
|
||||
# haystack: that instruction must not be linked from AGENTS.md or a
|
||||
# skill (automatic pickup), though a CONTRACT.md mentioning it by name
|
||||
@@ -441,7 +497,7 @@ def verify():
|
||||
"Link it from a skill, a contract, AGENTS.md, or CLAUDE.md, or delete it."
|
||||
)
|
||||
|
||||
# 5. instructions/dev/ is a hard boundary: `dist export` prunes it whole,
|
||||
# 6. instructions/dev/ is a hard boundary: `dist export` prunes it whole,
|
||||
# so nothing outside it may depend on something inside it staying
|
||||
# around in a distributed instance. See dev_only_forbidden_references's
|
||||
# docstring for the dist:strip exemption.
|
||||
|
||||
@@ -533,6 +533,72 @@ def test_no_shipped_document_cites_an_issue():
|
||||
assert docs_verify.check_no_issue_references() == []
|
||||
|
||||
|
||||
def test_a_dead_relative_link_is_reported(tmp_path, monkeypatch):
|
||||
"""Regression guard for the bug this check exists to catch: a `../` count
|
||||
wrong for the file's own depth is invisible to every other check - the
|
||||
name it links to is real, the text renders, and nothing resolves the
|
||||
target to notice it lands nowhere."""
|
||||
fake = tmp_path / "example.md"
|
||||
fake.write_text("See [tools/CONTRACT.md](../tools/CONTRACT.md) for the command table.\n", encoding="utf-8")
|
||||
monkeypatch.setattr(docs_verify.toc, "target_files", lambda: [fake])
|
||||
issues = docs_verify.check_reference_targets()
|
||||
assert len(issues) == 1
|
||||
assert "example.md:1" in issues[0]
|
||||
assert "../tools/CONTRACT.md" in issues[0]
|
||||
|
||||
|
||||
def test_a_resolving_relative_link_is_not_reported(tmp_path, monkeypatch):
|
||||
(tmp_path / "tools").mkdir()
|
||||
(tmp_path / "tools" / "CONTRACT.md").write_text("# Contract\n", encoding="utf-8")
|
||||
fake = tmp_path / "example.md"
|
||||
fake.write_text("See [tools/CONTRACT.md](tools/CONTRACT.md) for the command table.\n", encoding="utf-8")
|
||||
monkeypatch.setattr(docs_verify.toc, "target_files", lambda: [fake])
|
||||
assert docs_verify.check_reference_targets() == []
|
||||
|
||||
|
||||
def test_an_absolute_url_is_not_resolved_as_a_path(tmp_path, monkeypatch):
|
||||
fake = tmp_path / "example.md"
|
||||
fake.write_text("See [Anthropic](https://www.anthropic.com).\n", encoding="utf-8")
|
||||
monkeypatch.setattr(docs_verify.toc, "target_files", lambda: [fake])
|
||||
assert docs_verify.check_reference_targets() == []
|
||||
|
||||
|
||||
def test_a_section_anchor_is_stripped_before_resolving(tmp_path, monkeypatch):
|
||||
"""CommonMark anchors are not filesystem paths - only the path part of
|
||||
`target#anchor` is checked for existence."""
|
||||
(tmp_path / "kb").mkdir()
|
||||
(tmp_path / "kb" / "CONVENTIONS.md").write_text("## Tone\n", encoding="utf-8")
|
||||
fake = tmp_path / "example.md"
|
||||
fake.write_text("See [kb/CONVENTIONS.md § Tone](kb/CONVENTIONS.md#tone).\n", encoding="utf-8")
|
||||
monkeypatch.setattr(docs_verify.toc, "target_files", lambda: [fake])
|
||||
assert docs_verify.check_reference_targets() == []
|
||||
|
||||
|
||||
def test_link_syntax_shown_as_an_example_in_a_fence_is_not_flagged(tmp_path, monkeypatch):
|
||||
"""A passage documenting bad link syntax must not be mistaken for a real
|
||||
reference - code fences are masked before scanning, mirroring `toc.py`."""
|
||||
fake = tmp_path / "example.md"
|
||||
fake.write_text(
|
||||
"Do not write it like this:\n\n```markdown\n[gates.md](../nonexistent.md)\n```\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
monkeypatch.setattr(docs_verify.toc, "target_files", lambda: [fake])
|
||||
assert docs_verify.check_reference_targets() == []
|
||||
|
||||
|
||||
def test_every_reference_files_link_targets_resolve():
|
||||
"""Forward direction, against the real tree: every relative link in a file
|
||||
`toc.target_files()` covers must resolve - this is what a `../` count
|
||||
wrong for the file's own depth is caught by."""
|
||||
assert docs_verify.check_reference_targets() == []
|
||||
|
||||
|
||||
def test_verify_raises_when_a_reference_target_is_dead(monkeypatch):
|
||||
monkeypatch.setattr(docs_verify, "check_reference_targets", lambda: ["dangling"])
|
||||
with pytest.raises(typer.Exit):
|
||||
docs_verify.verify()
|
||||
|
||||
|
||||
def test_a_cited_issue_number_is_reported(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
docs_verify,
|
||||
|
||||
@@ -341,6 +341,123 @@ def test_a_self_mention_does_not_count_as_a_reference(layer):
|
||||
assert "gates.md" not in instructions_cmd.referenced_names()
|
||||
|
||||
|
||||
# --- skill reference paths ---------------------------------------------------
|
||||
|
||||
|
||||
def test_the_fixture_skill_carries_no_relative_link(layer):
|
||||
"""Forward direction: the fixture's own `wiki-demo/SKILL.md` mentions
|
||||
`gates.md` as a bare word (`referenced_names()` relies on exactly that
|
||||
substring match), never as a markdown link - so it must not trip the ban."""
|
||||
assert instructions_cmd.check_skill_reference_paths() == []
|
||||
|
||||
|
||||
def test_a_relative_markdown_link_in_a_skill_is_reported(layer):
|
||||
"""`sync` copies `wiki-demo/SKILL.md` to `.claude/skills/wiki-demo/SKILL.md`
|
||||
and `.agents/skills/wiki-demo/SKILL.md` - a different depth than the
|
||||
source - so a link written `../gates.md` here would resolve to a
|
||||
different, usually nonexistent, file once published."""
|
||||
(layer / "instructions" / "wiki-demo" / "SKILL.md").write_text(
|
||||
"---\nname: wiki-demo\ndescription: Demo skill.\n---\n\n"
|
||||
"# Demo\n\nSee [gates.md](../gates.md).\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
issues = instructions_cmd.check_skill_reference_paths()
|
||||
assert len(issues) == 1
|
||||
assert "wiki-demo" in issues[0] and "../gates.md" in issues[0]
|
||||
|
||||
|
||||
def test_a_plain_root_relative_path_is_not_a_reported_link(layer):
|
||||
"""The decided fix: a `SKILL.md` names its target as a repo-root-relative
|
||||
plain path, not a link - even one written correctly with brackets and
|
||||
parens. `instructions/gates.md` on its own, with no `[...]`, must pass."""
|
||||
(layer / "instructions" / "wiki-demo" / "SKILL.md").write_text(
|
||||
"---\nname: wiki-demo\ndescription: Demo skill.\n---\n\n"
|
||||
"# Demo\n\nSee `instructions/gates.md`.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert instructions_cmd.check_skill_reference_paths() == []
|
||||
|
||||
|
||||
def test_an_absolute_url_in_a_skill_is_not_reported(layer):
|
||||
"""The ban is about relative paths breaking under the copy - an external
|
||||
link is not affected by where the file sits, so it stays a normal link."""
|
||||
(layer / "instructions" / "wiki-demo" / "SKILL.md").write_text(
|
||||
"---\nname: wiki-demo\ndescription: Demo skill.\n---\n\n"
|
||||
"# Demo\n\nSee [the spec](https://example.com/spec).\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert instructions_cmd.check_skill_reference_paths() == []
|
||||
|
||||
|
||||
def test_a_pure_anchor_link_in_a_skill_is_not_reported(layer):
|
||||
"""A same-page `#anchor` link is not a filesystem reference and does not
|
||||
move when the file is copied."""
|
||||
(layer / "instructions" / "wiki-demo" / "SKILL.md").write_text(
|
||||
"---\nname: wiki-demo\ndescription: Demo skill.\n---\n\n"
|
||||
"# Demo\n\n## Steps\n\nSee [Steps](#steps) above.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert instructions_cmd.check_skill_reference_paths() == []
|
||||
|
||||
|
||||
def test_link_syntax_shown_as_an_example_in_a_fence_is_not_flagged(layer):
|
||||
"""A skill documenting the banned syntax as an example (rather than using
|
||||
it) must not trip its own ban - code fences are masked before scanning,
|
||||
the same way `toc.py` masks them before hunting for headings."""
|
||||
(layer / "instructions" / "wiki-demo" / "SKILL.md").write_text(
|
||||
"---\nname: wiki-demo\ndescription: Demo skill.\n---\n\n"
|
||||
"# Demo\n\nDo not write it like this:\n\n"
|
||||
"```markdown\n[gates.md](../gates.md)\n```\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
assert instructions_cmd.check_skill_reference_paths() == []
|
||||
|
||||
|
||||
def test_verify_rejects_a_skill_with_a_relative_markdown_link(layer):
|
||||
(layer / "instructions" / "wiki-demo" / "SKILL.md").write_text(
|
||||
"---\nname: wiki-demo\ndescription: Demo skill.\n---\n\n"
|
||||
"# Demo\n\nSee [gates.md](../gates.md).\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
instructions_cmd.sync(force=False)
|
||||
with pytest.raises(typer.Exit):
|
||||
instructions_cmd.verify()
|
||||
|
||||
|
||||
def test_a_plain_path_resolves_the_same_regardless_of_which_copy_reads_it(layer):
|
||||
"""The property the whole fix rests on. A repo-root-relative plain path
|
||||
(`instructions/gates.md`) names the same file whether it is read from the
|
||||
source (`instructions/wiki-demo/SKILL.md`) or from either published copy
|
||||
(`.claude/skills/wiki-demo/SKILL.md`, `.agents/skills/wiki-demo/SKILL.md`),
|
||||
because it is resolved against the instance root, never against the
|
||||
reading file's own directory.
|
||||
|
||||
A relative link has no such property, which is the defect this whole
|
||||
check exists to prevent: the same `../gates.md` means
|
||||
`instructions/gates.md` from the source but a nonexistent
|
||||
`.claude/skills/gates.md` / `.agents/skills/gates.md` from either
|
||||
published copy - one directory short of the real file, because `sync`
|
||||
copies the skill one level shallower than `instructions/<name>/` sits."""
|
||||
instructions_cmd.sync(force=False)
|
||||
target = layer / "instructions" / "gates.md"
|
||||
assert target.is_file()
|
||||
|
||||
source_dir = layer / "instructions" / "wiki-demo"
|
||||
claude_copy_dir = _skill_copy(layer, ".claude")
|
||||
agents_copy_dir = _skill_copy(layer, ".agents")
|
||||
|
||||
# The plain path resolves against the instance root, regardless of which
|
||||
# of the three directories above is doing the reading.
|
||||
for _ in (source_dir, claude_copy_dir, agents_copy_dir):
|
||||
assert (layer / "instructions" / "gates.md").is_file()
|
||||
|
||||
# The equivalent relative link would not: correct from the source, wrong
|
||||
# from both copies.
|
||||
assert (source_dir / ".." / "gates.md").resolve() == target.resolve()
|
||||
assert not (claude_copy_dir / ".." / "gates.md").resolve().is_file()
|
||||
assert not (agents_copy_dir / ".." / "gates.md").resolve().is_file()
|
||||
|
||||
|
||||
# --- instructions/dev/ boundary ----------------------------------------------
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user