From 0fb8fd61224421c971d2a96d96e63927ad90d39e Mon Sep 17 00:00:00 2001 From: Torben Nehmer Date: Sat, 12 Sep 2026 23:21:45 +0200 Subject: [PATCH] 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 --- CHANGES.md | 26 ++++- VERSION | 2 +- instructions/CONTRACT.md | 38 +++++- instructions/dev/doc-pull-through.md | 4 +- instructions/dev/stack-close/SKILL.md | 14 +-- instructions/dev/stack-dev/SKILL.md | 30 ++--- instructions/wiki-ingest/SKILL.md | 32 ++--- instructions/wiki-lint/SKILL.md | 10 +- instructions/wiki-manage/SKILL.md | 18 +-- instructions/wiki-query/SKILL.md | 8 +- instructions/wiki-status/SKILL.md | 4 +- tools/CONTRACT.md | 8 +- tools/chemenu/commands/docs_verify.py | 75 +++++++++++- tools/chemenu/commands/instructions_cmd.py | 68 ++++++++++- tools/chemenu/tests/test_docs_verify.py | 66 +++++++++++ tools/chemenu/tests/test_instructions_cmd.py | 117 +++++++++++++++++++ 16 files changed, 442 insertions(+), 78 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ddd3cfb..c9dfe5c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -52,15 +52,20 @@ concern - readable here, never shipped as something to parse. --- -## 5.1.1-beta.1 - 2026-09-12 - gates.md/session-setup.md: die Budget-Ausnahme von version regrade haengt an der Aufrufform +## 5.1.1-beta.2 - 2026-09-12 - SKILL.md: relative Links durch repo-root-relative Pfade ersetzt, docs verify/instructions verify pruefen Linkziele **Author:** Torben Nehmer +**High impact** +- SKILL.md: relative Links durch repo-root-relative Pfade ersetzt, docs verify/instructions verify pruefen Linkziele + **Low impact** - gates.md/session-setup.md: die Budget-Ausnahme von version regrade haengt an der Aufrufform +### gates.md/session-setup.md: die Budget-Ausnahme von version regrade haengt an der Aufrufform + Doku-Nachzug zu `5.1.0`. Beide Dateien beschrieben die Budget-Ausnahme als feste Liste pro Kommandoname ("fixed allowlist"); `version regrade` ist die erste Ausnahme, die nur in *einer* Aufrufform liest - bar listet sie, mit Positionen schreibt sie `CHANGES.md`. Die Liste selbst @@ -68,6 +73,25 @@ bleibt an ihrem einen Ort (`tools/CONTRACT.md`), beide Stellen benennen jetzt ab Eintrag pro Aufruf statt pro Namen gilt. Aufgefallen in der Schlussphase derselben Arbeit, deshalb ein eigener Patch-Bump: der Pfad `instructions/` liegt im Version-Gate der CI. +### SKILL.md: relative Links durch repo-root-relative Pfade ersetzt, docs verify/instructions verify pruefen Linkziele + +`instructions sync` kopiert jede `SKILL.md` byteidentisch in `.agents/skills/` und +`.claude/skills/` - eine andere Verzeichnistiefe als die Quelle, ohne deren Nachbardateien. 52 von +58 relativen Markdown-Links in den sieben Skills zeigten deshalb in der publizierten Kopie ins +Leere, unbemerkt, weil kein Check je ein Linkziel gelesen hat (Gitea-Meldung: ein `session-setup.md`-Read +schlug in einer ausgelieferten Instanz fehl). Alle 58 Links sind jetzt repo-root-relative +Klartextpfade (`instructions/session-setup.md` statt `[session-setup.md](../session-setup.md)`) - +sie ueberleben die Kopie unveraendert, weil sie nicht von der Position der lesenden Datei abhaengen. +`instructions/CONTRACT.md` § "A skill's outbound reference is a plain path, not a link" traegt die +Regel. + +Zwei neue mechanische Checks verhindern das Wiederauftreten: `instructions verify` verbietet jeden +relativen Markdown-Link in einer `SKILL.md` (`check_skill_reference_paths`), `docs verify` loest +jeden relativen Link in den flachen Instructions und Contracts gegen den Arbeitsbaum auf +(`check_reference_targets`, ueber denselben Dateisatz wie `docs toc`). Nebenbei behoben: +`instructions/dev/doc-pull-through.md` hatte zwei Links mit falscher `../`-Tiefe, unabhaengig vom +Skill-Copy-Problem. + --- ## 5.1.0 - 2026-09-12 - changelog: Kandidaten-Eintrag nach Impact gruppiert, version regrade zur Nachkorrektur, version release verlangt eine Zusammenfassung diff --git a/VERSION b/VERSION index e28d5cc..9e9bc26 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.1.1-beta.1 +5.1.1-beta.2 diff --git a/instructions/CONTRACT.md b/instructions/CONTRACT.md index 515be46..e614d99 100644 --- a/instructions/CONTRACT.md +++ b/instructions/CONTRACT.md @@ -18,6 +18,7 @@ alongside [AGENTS.md](../AGENTS.md). - [Publishing](#publishing) - [Writing an instruction](#writing-an-instruction) - [A skill's H1 is a name, not an imperative](#a-skills-h1-is-a-name-not-an-imperative) + - [A skill's outbound reference is a plain path, not a link](#a-skills-outbound-reference-is-a-plain-path-not-a-link) - [Reference depth: bundled files, not repo-wide contracts](#reference-depth-bundled-files-not-repo-wide-contracts) - [When a skill carries a copy-in checklist](#when-a-skill-carries-a-copy-in-checklist) - [How much reasoning a step may carry](#how-much-reasoning-a-step-may-carry) @@ -190,6 +191,35 @@ exception in the same breath - "for promoted skills, the skill name is the title That is the whole exception. Everything else in this section binds a `SKILL.md` exactly as it binds an instruction. +### A skill's outbound reference is a plain path, not a link + +`tools/wikitool instructions sync` copies each `SKILL.md` byte for byte into +`.agents/skills//` and `.claude/skills//` (§ Publishing, above) - a different depth +than the source, and without the sibling files a relative link might expect. A markdown link +correct at `instructions//SKILL.md` (`../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/` does not reach the same target from +`.claude/skills/`. Fifty-two of the fifty-eight relative links across this repo's seven skills +broke exactly this way before this rule existed, silently - nothing rendered the copy to notice, +and no check read a link target. + +So a `SKILL.md` never writes an outbound reference as a relative markdown link, correct depth or +not. It names the target as a repo-root-relative **plain path** instead - `` `instructions/session-setup.md` ``, not `[session-setup.md](../session-setup.md)`; `` `kb/CONTRACT.md` `` for a +whole file, `` `kb/CONVENTIONS.md` § Tone `` for a section rather than an anchored link. The path +survives the copy unchanged because it does not depend on where the reading file sits: an +agent's working directory is the instance root regardless of which published copy it opened, so +the same plain path resolves in the source and in both published copies alike. The cost is that +the reference is no longer clickable from the source file - accepted deliberately, because the +source is not where an agent reads it from; the harness reads the published copy. +`tools/wikitool instructions verify` enforces the ban mechanically +(`check_skill_reference_paths`). + +This binds only `SKILL.md`. The flat `instructions/.md` form - this file included - is +never copied anywhere, so its relative links stay exactly as correct as their `../` count says, +and stay ordinary links; `tools/wikitool docs verify` (`check_reference_targets`) resolves those +against the working tree instead of banning the syntax, over the same reference-file scope +`tools/wikitool docs toc` uses. + ### Reference depth: bundled files, not repo-wide contracts Anthropic's skill-authoring guidance asks that reference files stay **one level deep from @@ -201,9 +231,11 @@ That rule governs **skill-bundled** material: files sitting in `instructions//CONTRACT.md` | | A rule, gate, or invariant `AGENTS.md` itself states | The relevant `AGENTS.md` section (Invariants, Gates, File naming, Routing, ...) | | A workflow, stage, or command a human operates by hand | Whichever of `README.md`, `EVALS.md`, `tools/README.md`, `INSTALL.md`, `DEVELOPMENT.md` names it - AGENTS.md § File naming says which document is for which reader | @@ -42,7 +42,7 @@ touched; a row that does not apply needs no action. 3. **Do not re-derive what `docs verify` already checks mechanically** - existence, table-row membership, ignore-canary state. That enumeration lives once, in - [tools/CONTRACT.md](../tools/CONTRACT.md)'s own `docs verify` row; copying it here would be a + [tools/CONTRACT.md](../../tools/CONTRACT.md)'s own `docs verify` row; copying it here would be a second copy that drifts, the exact failure this instruction exists to describe (Gitea #90). This instruction is only about the prose no check reads. diff --git a/instructions/dev/stack-close/SKILL.md b/instructions/dev/stack-close/SKILL.md index f4e977d..944192d 100644 --- a/instructions/dev/stack-close/SKILL.md +++ b/instructions/dev/stack-close/SKILL.md @@ -13,11 +13,11 @@ there and hands off here rather than continuing into this phase in the same brea printed its stack-machinery note ("this publish touched stack machinery...") and nothing has closed the work package it belongs to yet; or a package was published in an earlier session and never went through this skill (the gap this split exists to make impossible to skip past -silently - see [issue-tracking.md](../issue-tracking.md)'s note that a closed body is the version +silently - see `instructions/dev/issue-tracking.md`'s note that a closed body is the version everyone reads afterwards and nobody revisits). **This directory is dev-only.** Same boundary as `stack-dev` -([its own note](../stack-dev/SKILL.md) has the full reasoning) - `dist export` prunes +(its own `instructions/dev/stack-dev/SKILL.md` has the full reasoning) - `dist export` prunes `instructions/dev/` wholesale, so this skill never reaches a distributed instance. ## Why this is a separate skill, not `stack-dev`'s step 6 @@ -25,13 +25,13 @@ everyone reads afterwards and nobody revisits). The two phases around the mechanical middle of a stack-dev session have no mechanical guard at all - `pytest`, `docs verify` and `instructions verify` cover the code and tests in between, and nothing covers a changelog entry's accuracy, a `docs/` page's staleness, or an issue body's final -state (see [docs/model-and-effort-selection.md](../../../docs/model-and-effort-selection.md)). Asking the +state (see `docs/model-and-effort-selection.md`). Asking the same session to notice it has crossed into that second unchecked stretch - as a prose break inside `stack-dev`'s own step 6 - failed twice in a row on this stack (Gitea #42, then #30): both times the session knew the rule and skipped past it anyway, because nothing in the moment forced the question. Splitting the phase into its own skill does not add a check either - `wikitool` still does not know this tracker exists and must not learn (see -[issue-tracking.md](../issue-tracking.md) § What no tool checks) - but it removes the thing that +`instructions/dev/issue-tracking.md` § What no tool checks) - but it removes the thing that was actually failing: the closing *procedure* is no longer sitting in the session's context as a next step to run past - it exists only inside a skill someone has to invoke. @@ -66,7 +66,7 @@ and a fresh subagent starts without the session's context). - what was verified is named - which checks ran, which CI run - not a commit hash alone Then one short comment naming what changed against the previous state, and nothing else - - [issue-tracking.md](../issue-tracking.md) steps 2-3 and 7 have the full shape; this is that + `instructions/dev/issue-tracking.md` steps 2-3 and 7 have the full shape; this is that procedure, run at the point this skill exists to guarantee it actually gets run. **A closing report in a comment does not satisfy this**, however thorough: it reads as @@ -80,7 +80,7 @@ and a fresh subagent starts without the session's context). naming) - the same is true of `tools/CONTRACT.md`'s two tables and any touched `/CONTRACT.md`, whose prose `docs verify` checks only for presence and table-row membership, never for what a cell or a section actually says - ([doc-pull-through.md](../doc-pull-through.md)); of `README.md`/`INSTALL.md`/`DEVELOPMENT.md` + (`instructions/dev/doc-pull-through.md`); of `README.md`/`INSTALL.md`/`DEVELOPMENT.md` prose; and of a new instruction's own wording, which `instructions verify` checks structurally but never for what it claims. If the change this package shipped moved the reasoning or the behaviour one of these documents describes, update it now; if none did, say so rather than @@ -110,7 +110,7 @@ and a fresh subagent starts without the session's context). - **The work package spans several sessions?** Run this skill once, at the point the package is actually finished and its last publish has landed - not after every individual publish. A package still open across sessions keeps its body current per - [issue-tracking.md](../issue-tracking.md) step 2 in the meantime; that is maintenance, not + `instructions/dev/issue-tracking.md` step 2 in the meantime; that is maintenance, not closing. - **Resuming a package whose publish landed in an earlier, already-ended session?** Run this skill now, on whatever model the current session is - do not reopen the earlier session to run diff --git a/instructions/dev/stack-dev/SKILL.md b/instructions/dev/stack-dev/SKILL.md index c820187..2c5deae 100644 --- a/instructions/dev/stack-dev/SKILL.md +++ b/instructions/dev/stack-dev/SKILL.md @@ -39,30 +39,30 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li 1. **Confirm the mode.** If the task is ambiguous between "extend the tool" and "operate the wiki", ask rather than guess - the two have different rules for the same directories. 2. **Consult `instructions/dev/` for the concrete procedure.** Currently: - [commonplace-kb.md](../commonplace-kb.md) - vendored knowledge base on agent context + `instructions/dev/commonplace-kb.md` - vendored knowledge base on agent context engineering, memory and deploy-time learning; consult before a design decision in those areas. - [issue-tracking.md](../issue-tracking.md) - open work lives in Gitea issues, one per work + `instructions/dev/issue-tracking.md` - open work lives in Gitea issues, one per work package, labelled `area/`, `kind/`, `prio/` and `size/`. There is no `TODO.md`. **The body of the issue you are working on is this session's plan file:** keep it current as the state moves, so an interrupted session leaves a body the next one can resume from, *and* rewrite it to its final state before closing. Both halves bind; the second is what - [`stack-close`](../stack-close/SKILL.md) carries out once this skill's own work is published - + `stack-close` (`instructions/dev/stack-close/SKILL.md`) carries out once this skill's own work is published - see step 6 below. An issue labelled `status/incoming` is the exception to all of that: it is a human's stub, not a spec, and it is **never implemented as it stands** - it gets worked out and triaged first. Read this file before filing something for later, before editing or closing an issue, before picking up an incoming stub, or before deciding what to pick up next. - [testing-conventions.md](../testing-conventions.md) - the suite runs against a deliberately + `instructions/dev/testing-conventions.md` - the suite runs against a deliberately empty machine; what the autouse fixture already neutralizes, and what a test still has to establish itself. Read it before adding or changing a test. - [version-parts.md](../version-parts.md) - which part a change bumps: the drop-in test, the + `instructions/dev/version-parts.md` - which part a change bumps: the drop-in test, the catalogue of breaks that cross the compatibility boundary with `kb/` untouched, and what to put in front of the user before a breaking bump. Read it before step 4. - [corpus-policy.md](../corpus-policy.md) - what "curated enough" means for the shared + `instructions/dev/corpus-policy.md` - what "curated enough" means for the shared demo/testbed `kb/`, the measurable floors that define it, and what a reactive fix may and may not do to corpus content. Read it before judging whether the corpus can exercise a change, or before any fix that would touch `kb/` content. - [doc-pull-through.md](../doc-pull-through.md) - which document makes a claim about a touched + `instructions/dev/doc-pull-through.md` - which document makes a claim about a touched surface (a `wikitool` command, a stage's rules, an `AGENTS.md` rule/gate/invariant, a README-shaped human doc, a `docs/` page's reasoning) and therefore needs updating alongside the code, since `docs verify` never reads a cell's prose. Read it before step 6. @@ -100,7 +100,7 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li Effort is the cheaper lever than the model, and `high` is the floor for anything touching more than one file or a contract. Full table and reasoning: - [docs/model-and-effort-selection.md](../../../docs/model-and-effort-selection.md). + `docs/model-and-effort-selection.md`. 4. **Raise the version, if the change ships.** A change under `tools/`, `types/`, `instructions/`, `AGENTS.md` or a `CONTRACT.md` reaches every future instance, so it needs a @@ -113,7 +113,7 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li `--impact high|medium|low` (default `medium`) grades this bump in the changelog entry's own list - `tools/wikitool version regrade` corrects it later if the candidate's overall shape changes the read on an earlier one; see - [instructions/dev/version-parts.md](../version-parts.md) § The candidate model. + `instructions/dev/version-parts.md` § The candidate model. Never edit `VERSION` or the entry's heading by hand - `bump` writes both, and `docs verify` fails a tree where they disagree. Pick the part by whether the new version is a **drop-in @@ -128,12 +128,12 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li Content migration is one way to land in the last row, not the definition of it: a rename of the update path, the artefact, an import name, a flag or an envvar breaks a swap with `kb/` entirely untouched. The full test, the catalogue of such breaks, and what to put in front of - the user first are in [version-parts.md](../version-parts.md) - **read it before choosing + the user first are in `instructions/dev/version-parts.md` - **read it before choosing `--major`.** A `--major` bump therefore needs two things recorded. `--breaking ""` is required on every boundary-crossing bump; on top of it, a migration document for the new - version - written per [migrate-corpus.md](../../migrate-corpus.md) - or + version - written per `instructions/migrate-corpus.md` - or `--no-migration ""` when no content actually has to change. `bump` refuses without either, and so does `docs verify`: an instance learning that it must migrate, with nothing telling it how, is a dead end. @@ -145,7 +145,7 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li do not need a bump - CI's version gate is scoped to what changes behaviour. 5. **Pull through every document that makes a claim about the surface you touched - `docs verify` - checks a cell's presence, never its prose.** [doc-pull-through.md](../doc-pull-through.md) has + checks a cell's presence, never its prose.** `instructions/dev/doc-pull-through.md` has the table of which document that is, per surface. 6. **Verify, then publish.** `tools/wikitool docs verify`, `tools/wikitool instructions verify`, @@ -162,7 +162,7 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li **This skill stops here.** The closing phase - rewriting the issue body to its final state, checking for `docs/` staleness, and naming which model ran which phase of the session - lives - in [`stack-close`](../stack-close/SKILL.md), not in a further step of this one. Invoke it now; + in `stack-close` (`instructions/dev/stack-close/SKILL.md`), not in a further step of this one. Invoke it now; do not fold its work into this session under this skill's rules, and do not treat "the change is published" as this work package being done. @@ -176,7 +176,7 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li user decides whether it is worth that: show them what breaks, what an instance has to do about it, and the alternatives (avoid the break with a shim, defer and batch it with the next one, or split it behind a deprecation window), then recommend one and wait for a go-ahead. - [version-parts.md](../version-parts.md) step 4 has the full shape. A surfacing boundary crossing + `instructions/dev/version-parts.md` step 4 has the full shape. A surfacing boundary crossing is also a reason to offer the model switch back up (step 3): the judgment it needs has no mechanical guard, and `docs verify` only checks that a crossing documents itself, never that the part was chosen correctly. @@ -186,4 +186,4 @@ stack development happens in the origin repo instead (see AGENTS.md's routing li Not for wiki content work - use `wiki-ingest`/`wiki-query`/`wiki-manage`/`wiki-lint`/ `wiki-status` for that. Not for setting up a new instance (`instructions/setup-instance.md`) or a fresh clone of this repo (`instructions/bootstrap.md`). Not for closing a work package after -its publish has landed - that is [`stack-close`](../stack-close/SKILL.md). +its publish has landed - that is `stack-close` (`instructions/dev/stack-close/SKILL.md`). diff --git a/instructions/wiki-ingest/SKILL.md b/instructions/wiki-ingest/SKILL.md index 28cafc4..54a2183 100644 --- a/instructions/wiki-ingest/SKILL.md +++ b/instructions/wiki-ingest/SKILL.md @@ -10,7 +10,7 @@ description: Process a new source file into the LLM wiki - extract entities and **Trigger:** User drops a file into `incoming/` (the normal path - see step 1) or directly into `raw/`, or explicitly requests ingestion. -**Before the first `wikitool` call:** [session-setup.md](../session-setup.md). +**Before the first `wikitool` call:** `instructions/session-setup.md`. Contracts are read **when the step needs them**, not upfront: a source that produces no concept pages should never have cost the concept contract. Field-level requirements always come from @@ -40,7 +40,7 @@ validator complains - and the ticked list is the only record that they happened. ## Steps 1. **Promote from `incoming/` if that is where the file sits.** Read - [raw/CONTRACT.md](../../raw/CONTRACT.md) "Getting a file in" and "Capture fields" if you have + `raw/CONTRACT.md` "Getting a file in" and "Capture fields" if you have not this session - the directory and any bundling are computed, never chosen by hand, but the two capture flags are not: `raw accept` refuses without them. @@ -63,7 +63,7 @@ validator complains - and the ticked list is the only record that they happened. **A file that arrived through the MCP `submit` tool is not yet in `incoming/`** - it sits in `mcp-upload//`, a quarantine no command in this step reads. A reviewer promotes it first with `wikitool upload accept --confirm `, per - [instructions/ingest-queue.md](../ingest-queue.md); once accepted it is an ordinary file in + `instructions/ingest-queue.md`; once accepted it is an ordinary file in `incoming/` and this step applies to it exactly as to anything dropped there by hand. **If this refuses because the name is already claimed** (a file stem or a bundle directory @@ -79,7 +79,7 @@ validator complains - and the ticked list is the only record that they happened. **Check the size first, on both axes.** *Volume* - how many raw files this ingest covers - and *breadth* - how many entities and concepts this one source would produce or update. - Either one past the thresholds in [ingest-large-tree.md](../ingest-large-tree.md) § When to + Either one past the thresholds in `instructions/ingest-large-tree.md` § When to run is that procedure, not this one: stop and follow it. There, volume is cut into units; breadth cannot be cut at all (`raw/` keeps a file whole, and one raw file has one owning source page) and buys an extract pass instead, before any page is written. Skipping either @@ -107,7 +107,7 @@ validator complains - and the ticked list is the only record that they happened. which entities/concepts to create or update, any specific emphasis. 6. **Create the source page.** Read - [kb/sources/COLLECTION.md](../../kb/sources/COLLECTION.md) first - it holds what this + `kb/sources/COLLECTION.md` first - it holds what this instance expects of a source page's sections and how it names one. ```bash @@ -140,18 +140,18 @@ validator complains - and the ticked list is the only record that they happened. article also pass `--set source_url=`; `raw_files:` must still point at the local copy. Then write the Summary / Key Takeaways / Action Items prose from step 5 - in the KB language, whatever the source's own language is, quoting verbatim passages in the - original. Which language that is: [kb/CONVENTIONS.md](../../kb/CONVENTIONS.md#language). + original. Which language that is: `kb/CONVENTIONS.md` § Language. What is exempt from it, in any language: - [kb/CONTRACT.md](../../kb/CONTRACT.md#language-and-identifiers). + `kb/CONTRACT.md` § Language and identifiers. Fill `## Not Extracted` in the same pass: what you read and deliberately did not promote, with the reason. Nothing in the repository can re-derive that judgment, and without it the same source gets re-litigated on the next pass. 7. **Create or update entity pages.** Read - [kb/entities/COLLECTION.md](../../kb/entities/COLLECTION.md) and - [kb/CONTRACT.md](../../kb/CONTRACT.md) plus - [kb/CONVENTIONS.md](../../kb/CONVENTIONS.md) first - the second is where provenance and + `kb/entities/COLLECTION.md` and + `kb/CONTRACT.md` plus + `kb/CONVENTIONS.md` first - the second is where provenance and citation are defined, the third where this instance's tone and naming forms are. **A subject earns a page when the source carries material for one.** A name the source @@ -186,7 +186,7 @@ validator complains - and the ticked list is the only record that they happened. 8. **Create or update concept pages** - only if the source produced any. Same pattern, including step 7's rule about which subjects earn a page at all, reading - [kb/concepts/COLLECTION.md](../../kb/concepts/COLLECTION.md) first: + `kb/concepts/COLLECTION.md` first: ```bash tools/wikitool new concept --name "" \ @@ -211,7 +211,7 @@ validator complains - and the ticked list is the only record that they happened. The new raw file(s) must no longer be listed as uncovered, and no `raw_files:` entry may be broken. -11. **Close out.** Follow [publish-cycle.md](../publish-cycle.md) with `--op ingest` and a +11. **Close out.** Follow `instructions/publish-cycle.md` with `--op ingest` and a message of the form `ingest: `. 12. **Check the lint cadence.** @@ -230,16 +230,16 @@ validator complains - and the ticked list is the only record that they happened. Two pages on one subject is the failure this step exists to prevent. - **One source names far more subjects than usual?** That is breadth, not volume. It is not split into several sources - it cannot be - and it does not get a page per name either: - [ingest-large-tree.md](../ingest-large-tree.md) § A broad source is not cut. + `instructions/ingest-large-tree.md` § A broad source is not cut. - **No raw file backs a claim you want to write?** Leave it out, or mark the page `provenance: mixed` and put it under `## General Guidance (unsourced)`. - **`publish` exited 42?** A single ingest is normally well under the Mass-Update Gate threshold. If it trips - a source touching many entities - show the user the output and stop; - see [gates.md](../gates.md). -- **A gate or the loop-breaker refuses anything?** Stop and follow [gates.md](../gates.md). + see `instructions/gates.md`. +- **A gate or the loop-breaker refuses anything?** Stop and follow `instructions/gates.md`. A multi-tool ingest should land in roughly 20-35 `wikitool` calls; needing far more is a sign the source should be split into several ingests - which is - [ingest-large-tree.md](../ingest-large-tree.md), not a bigger budget. + `instructions/ingest-large-tree.md`, not a bigger budget. ## wikitool commands used diff --git a/instructions/wiki-lint/SKILL.md b/instructions/wiki-lint/SKILL.md index 2392b7b..eb9cfb9 100644 --- a/instructions/wiki-lint/SKILL.md +++ b/instructions/wiki-lint/SKILL.md @@ -11,7 +11,7 @@ description: Health-check the LLM wiki - broken links, orphan pages, uncovered r threshold reached - `wiki-ingest`'s last step checks it after every publish, so the count is never something an agent has to remember. -**Before the first `wikitool` call:** [session-setup.md](../session-setup.md). +**Before the first `wikitool` call:** `instructions/session-setup.md`. ## Run checklist @@ -51,7 +51,7 @@ mechanical half looks exactly like a complete one. The *Redundant see-also* section is the one that looks mechanical and is not - do **not** clear it under step 7. It names a `see-also` edge standing beside a specific label on the reverse direction, and the obvious repair destroys the thing worth keeping: `xref remove` - clears the reference in *both* directions (see [tools/CONTRACT.md](../../tools/CONTRACT.md)), + clears the reference in *both* directions (see `tools/CONTRACT.md`), so removing the weak edge takes the labelled one with it and the pair ends up saying nothing at all. Either relabel the weak edge to something true with `xref add`, which only ever touches the source page, or leave it and report it at step 9. Clearing a batch of these is a @@ -90,7 +90,7 @@ mechanical half looks exactly like a complete one. 7. **Repair what is mechanical.** A dangling frontmatter reference is either a page that should exist (`tools/wikitool new ...`) or a reference that should not (`tools/wikitool xref remove --a "" --b ""`). A title that changed is - `tools/wikitool rename` - see [page-lifecycle.md](../page-lifecycle.md). Never hand-edit a + `tools/wikitool rename` - see `instructions/page-lifecycle.md`. Never hand-edit a frontmatter array to clear one. 8. **Verify the stack.** @@ -129,7 +129,7 @@ mechanical half looks exactly like a complete one. - **Publish?** Lint does not auto-publish. Run `tools/wikitool publish` only if asked. - **Bulk fixes touched 10+ files?** Expected for a lint pass: `publish` exits 42. Show the - user its output and stop; see [gates.md](../gates.md). Consider `--path` batches instead. + user its output and stop; see `instructions/gates.md`. Consider `--path` batches instead. - **The gate or loop-breaker keeps tripping?** That is a signal to stop and re-plan with the user, not to pass `--override-budget`. A full pass should land in roughly 20-35 calls. @@ -140,7 +140,7 @@ mechanical half looks exactly like a complete one. `publish` (only if asked) **Deliberately absent:** `rm` - a lint pass never deletes a page, and -[page-lifecycle.md](../page-lifecycle.md) is where a deletion belongs. `log status` - it decides +`instructions/page-lifecycle.md` is where a deletion belongs. `log status` - it decides this skill's *trigger*, but `wiki-ingest`'s last step is what runs it. ## Output diff --git a/instructions/wiki-manage/SKILL.md b/instructions/wiki-manage/SKILL.md index 75363a9..e910059 100644 --- a/instructions/wiki-manage/SKILL.md +++ b/instructions/wiki-manage/SKILL.md @@ -11,11 +11,11 @@ catalog and the audit log in sync. **Trigger:** User requests a new entity/concept/comparison page, or new information needs integrating into an existing one. -**Before the first `wikitool` call:** [session-setup.md](../session-setup.md). +**Before the first `wikitool` call:** `instructions/session-setup.md`. -**Read before drafting:** [kb/CONTRACT.md](../../kb/CONTRACT.md) - linking and provenance, +**Read before drafting:** `kb/CONTRACT.md` - linking and provenance, both of which the tool enforces - and -[kb/CONVENTIONS.md](../../kb/CONVENTIONS.md), which is where this instance's language, naming +`kb/CONVENTIONS.md`, which is where this instance's language, naming forms, tone and relationship labels are, together with the target collection's own `COLLECTION.md`, which carries its quality goal and what is local to that subtree. Field-level requirements come from `tools/wikitool types describe `. @@ -48,7 +48,7 @@ requirements come from `tools/wikitool types describe `. subjects - so the prose connects to existing pages instead of restating them. 5. **Draft.** Fill in the generated skeleton's TODO sections, following the tone rules in - [kb/CONVENTIONS.md](../../kb/CONVENTIONS.md#tone). If `provenance:` is `sourced` or `mixed`, cite + `kb/CONVENTIONS.md` § Tone. If `provenance:` is `sourced` or `mixed`, cite hard facts as you write them with `tools/wikitool cite add --page "" --source "Source - X"`, which also adds `X` to `sources:` - paste the `[^cite-id]` marker it prints. @@ -60,7 +60,7 @@ requirements come from `tools/wikitool types describe <type>`. One per relationship. Never hand-edit `related:`. -7. **Close out.** [publish-cycle.md](../publish-cycle.md), `--op create`. +7. **Close out.** `instructions/publish-cycle.md`, `--op create`. ## Updating a page @@ -84,11 +84,11 @@ requirements come from `tools/wikitool types describe <type>`. Never hand-edit `modified:`, `summary:` or `provenance:`. -7. **Close out.** [publish-cycle.md](../publish-cycle.md), `--op update`. +7. **Close out.** `instructions/publish-cycle.md`, `--op update`. ## Renaming, deleting, or unlinking -That is [page-lifecycle.md](../page-lifecycle.md). A title is the wiki's only identifier for a +That is `instructions/page-lifecycle.md`. A title is the wiki's only identifier for a page, so none of it is a file operation. ## Decision points @@ -98,7 +98,7 @@ page, so none of it is a file operation. - **Entity or concept?** A thing you can point at is an entity; a *why* or *how* is a concept. The collection contracts draw the line. - **`publish` refused?** A single page is normally well under the threshold. If it trips, - [gates.md](../gates.md). + `instructions/gates.md`. ## wikitool commands used @@ -106,7 +106,7 @@ page, so none of it is a file operation. `sources rebuild-index`, `index rebuild`, `log append`, `publish` `xref remove` belongs to the unlinking case, which this skill delegates whole to -[page-lifecycle.md](../page-lifecycle.md) rather than describing in a step of its own. +`instructions/page-lifecycle.md` rather than describing in a step of its own. ## Output diff --git a/instructions/wiki-query/SKILL.md b/instructions/wiki-query/SKILL.md index 8c75da2..1f74fb0 100644 --- a/instructions/wiki-query/SKILL.md +++ b/instructions/wiki-query/SKILL.md @@ -9,7 +9,7 @@ description: Answer a question using the LLM wiki's compiled knowledge - read-on **Trigger:** User asks a question. -**Before the first `wikitool` call:** [session-setup.md](../session-setup.md). +**Before the first `wikitool` call:** `instructions/session-setup.md`. **Hard rule:** read-only with respect to wiki *content*. Never modify, hand-edit, or scaffold a page while answering. Two exceptions, both mechanical: step 6 (filing a valuable answer through @@ -46,7 +46,7 @@ invariant - rather than synthesizing a plausible-sounding answer from general kn 4. **Answer and cite.** Name the wiki pages the answer came from, and the sources behind them. Hedge to what those sources carry, not to a number - see - [kb/CONVENTIONS.md § Hedging](../../kb/CONVENTIONS.md#hedging). + `kb/CONVENTIONS.md` § Hedging. 5. **Decide what earns a page - before the first `new`.** Name every page you are considering, then hold each one on its own against all three criteria: the answer required synthesis @@ -75,9 +75,9 @@ invariant - rather than synthesizing a plausible-sounding answer from general kn exist under different words. Then say the wiki has no confident source, and offer to ingest one. - **Filed a page?** Query does **not** auto-publish. Run `tools/wikitool publish` only if asked; - the sequence is in [publish-cycle.md](../publish-cycle.md). + the sequence is in `instructions/publish-cycle.md`. - **Several answers filed at once?** That can trip the Mass-Update Gate - see - [gates.md](../gates.md). The gate is a brake, not the check: it counts files and knows nothing + `instructions/gates.md`. The gate is a brake, not the check: it counts files and knows nothing about whether any of them earned a page. Step 5 is what decides that, and a batch small enough to pass the gate has not been cleared by it. diff --git a/instructions/wiki-status/SKILL.md b/instructions/wiki-status/SKILL.md index 4eea5a8..b16e321 100644 --- a/instructions/wiki-status/SKILL.md +++ b/instructions/wiki-status/SKILL.md @@ -10,14 +10,14 @@ semantic review a lint pass does. **Trigger:** User asks for wiki statistics, "what's new", or a quick health snapshot. -**Before the first `wikitool` call:** [session-setup.md](../session-setup.md) - step 2's `lint` is +**Before the first `wikitool` call:** `instructions/session-setup.md` - step 2's `lint` is not on the budget's exemption allowlist and is counted like any other call, gitignored report or not (§ Scope there). **Hard rule:** read-only with respect to wiki *content*. Never create, modify, or scaffold a page, never repair a finding, never publish. One file does get written: the report `lint` produces in step 2. That is not an exception being stretched - `reports/` is gitignored and holds -no wiki page ([reports/CONTRACT.md](../../reports/CONTRACT.md)), so the write leaves nothing +no wiki page (`reports/CONTRACT.md`), so the write leaves nothing behind that the wiki ships. If something looks wrong, point the user at `wiki-lint` or `wiki-manage` instead of fixing it here. diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index 5231f94..ce59a29 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -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 diff --git a/tools/chemenu/commands/docs_verify.py b/tools/chemenu/commands/docs_verify.py index 0f16abf..372c03b 100644 --- a/tools/chemenu/commands/docs_verify.py +++ b/tools/chemenu/commands/docs_verify.py @@ -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()}." ) diff --git a/tools/chemenu/commands/instructions_cmd.py b/tools/chemenu/commands/instructions_cmd.py index e01130f..84a08f8 100644 --- a/tools/chemenu/commands/instructions_cmd.py +++ b/tools/chemenu/commands/instructions_cmd.py @@ -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. diff --git a/tools/chemenu/tests/test_docs_verify.py b/tools/chemenu/tests/test_docs_verify.py index b5b77a3..9e286bd 100644 --- a/tools/chemenu/tests/test_docs_verify.py +++ b/tools/chemenu/tests/test_docs_verify.py @@ -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, diff --git a/tools/chemenu/tests/test_instructions_cmd.py b/tools/chemenu/tests/test_instructions_cmd.py index 381e4cf..0e39825 100644 --- a/tools/chemenu/tests/test_instructions_cmd.py +++ b/tools/chemenu/tests/test_instructions_cmd.py @@ -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 ----------------------------------------------