diff --git a/CHANGES.md b/CHANGES.md index 1054d74..71cc19d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -35,7 +35,7 @@ dev-checkout concern - readable here, never shipped as something to parse. --- -## 5.0.0-beta.12 - 2026-09-11 - AGENTS.md-Changelog-Absatz korrigiert: Contract-Prosa ist Sitzungsarbeit, doc-pull-through-Instruction ergaenzt +## 5.0.0-beta.13 - 2026-09-11 - docs verify prueft die Kommando- und Fehlerkontrakttabelle in tools/CONTRACT.md getrennt, 10 fehlende Fehlerkontrakt-Zeilen nachgetragen (schliesst #91) **Author:** Torben Nehmer @@ -65,6 +65,7 @@ dev-checkout concern - readable here, never shipped as something to parse. - tools/CONTRACT.md: raw accept Doku auf Datums-Shard und Capture-Felder nachgezogen (schliesst #89) - MCP submit-Tool: Quarantäne-Schreibpfad mit Upload Review Gate (schliesst #32) - AGENTS.md-Changelog-Absatz korrigiert: Contract-Prosa ist Sitzungsarbeit, doc-pull-through-Instruction ergaenzt +- docs verify prueft die Kommando- und Fehlerkontrakttabelle in tools/CONTRACT.md getrennt, 10 fehlende Fehlerkontrakt-Zeilen nachgetragen (schliesst #91) @@ -1217,6 +1218,49 @@ Geändert: `AGENTS.md`, `tools/chemenu/commands/docs_verify.py`, `tools/wikitool instructions verify` (nach `instructions sync`), volle `pytest`-Suite (1185 passed). Schließt #90. +**`docs verify` prüfte § Commands und § Error contracts in `tools/CONTRACT.md` als einen Topf +(#91).** `TABLE_CELL_RE` sammelte das erste gebacktickte Wort jeder Tabellenzeile über das ganze +Dokument, ohne Abschnittsgrenze - eine aus § Commands gelöschte Zeile fiel nicht auf, solange +derselbe Name noch in § Error contracts stand, und § Error contracts wurde gegen nichts +erzwungen. Gemessen am Baum vor diesem Fix (55 registrierte Kommandos): § Commands war +vollständig, § Error contracts fehlten zehn Zeilen - `budget reset`, `instructions list`, +`instructions verify`, `log append`, `migrate status`, `sources rebuild-index`, `sources trace`, +`types describe`, `upload show`, `version notes` - und `docs verify` blieb grün. + +`check_cli_readme()` liest die beiden Tabellen jetzt über ihre `##`-Überschrift ab (neue +`section_text()`), unabhängig voneinander und je Tabelle in beide Richtungen; fehlt eine der +beiden Überschriften, meldet die Funktion das explizit statt stillschweigend auf "ganzes +Dokument" zurückzufallen. Ursache der zehn Lücken war überwiegend eine zweite, bisher unsichtbare +Untugend: mehrere Kommandos teilten sich in § Error contracts eine Zeile der Form "`a` / `b`" - +nur das erste Backtick-Wort einer Zeile zählte je als "dokumentiert", der Rest der Gruppe war für +den (ungeprüften) Check unsichtbar. Behoben, indem jede betroffene Gruppe in eigene Zeilen mit +eigenem, gegen den Code nachgesehenem Inhalt aufgeteilt wurde - dabei fielen nebenbei drei +sachlich falsche Zeilen auf: `sources coverage`, `types list`, `instructions list` und `budget +status` schlagen nie fehl, die alte gemeinsame Zeile behauptete das Gegenteil, weil sie das +Verhalten des jeweils benachbarten Kommandos mit übernahm. Eine echte Formatierungslücke kam +dazu: `log append`s Zeile stand im Quelltext ohne Zeilenumbruch hinter der von `index rebuild` / +`sources rebuild-index` verschmolzen - für Menschen als Tabelle kaum lesbar und für die +zeilenanfang-verankerte Regex unsichtbar. + +**MINOR, kein neues Boundary-Crossing:** additiv und in beide Richtungen drop-in - eine +bestehende Instanz kopiert `docs_verify.py` und `tools/CONTRACT.md` über sich, ohne Hand-Arbeit +oder Migration. Eine Ausnahme ist es wert, genannt zu werden: eine private Instanz mit einem +lokal abweichenden `tools/CONTRACT.md` (`instructions/private-instance.md`) kann nach diesem +Update zum ersten Mal an der strengeren Prüfung scheitern, wenn ihr eigener Baum dieselbe +Fehlerkontrakt-Lücke trägt oder ihre Kommandotabelle anders benannte `##`-Überschriften +verwendet. Das ist kein Boundary-Crossing - kein Dateiformat ändert sich, keine bestehende +Funktion verschwindet -, sondern derselbe bereits akzeptierte Fall, den jede Verschärfung von +`docs verify` seit jeher mit sich bringt: der Fix legt eine bereits vorhandene +Dokumentationslücke bloß, statt eine neue Anforderung einzuführen. Der Kandidat trägt seine +`--breaking`-Zeile bereits aus einem früheren Bump; diese Änderung fügt keine neue hinzu. + +Geändert: `tools/chemenu/commands/docs_verify.py` (`section_text`, `check_cli_readme`), +`tools/CONTRACT.md` (die `docs verify`-Zeile sowie zehn aufgeteilte bzw. nachgetragene +Fehlerkontrakt-Zeilen), `tools/chemenu/tests/test_docs_verify.py` (neue Tests für +Abschnittstrennung, fehlende/umbenannte Überschrift, § Error contracts in beiden Richtungen). +Verifiziert: `tools/wikitool docs verify`, `tools/wikitool instructions verify`, volle +`pytest`-Suite (1193 passed). Schließt #91. + --- ## 4.7.4 - 2026-09-04 - bootstrap.md nennt den session-id-WARN nach frischem Bootstrap explizit als erwartet diff --git a/VERSION b/VERSION index 8777583..156930a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.0.0-beta.12 +5.0.0-beta.13 diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index f396cce..0e2ae97 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -86,7 +86,7 @@ tools/wikitool --help | `instructions sync [--force]` | Publish every `instructions//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 `` 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 here (and vice versa), 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 `` 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 `` 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 toc [--apply]` | Create, refresh or remove the generated table-of-contents region (`` ... ``, 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 | | `eval sessions [--json]` | List the sessions that have a trace under `reports/telemetry/`, most recent first. Read-only and exempt from the Iteration Budget Gate | | `eval score [--session ] [--json] [--markdown out.md] [--save] [--fail-on-error]` | Score one traced session: structural state from `lint`'s own checks (L1) plus trajectory rules over the trace (L2) - was a refused call repeated unchanged, was a gate flag passed without that gate having refused anything, did a publish of `kb/` pages go unlogged. Defaults to the current session. `--save` writes `reports/evals//.{json,md}`. Read-only over `kb/` and exempt from the budget; see [../EVALS.md](../EVALS.md) | @@ -206,7 +206,9 @@ is atomic, and whether a retry is safe. | `cite id` | Never fails | Read-only | Safe to retry freely | | `cite add` | Page or source not found | Yes - single file write | Safe to retry; upserting the same (page, source, file) pair twice reuses the existing id and changes nothing the second time | | `cite sync` | Neither or both of `--page`/`--all` given, or page not found | No - one write per page, each idempotent | Safe to retry freely. An undefined-reference report is not a failure - fix the reference (or run `cite add`) and re-run | -| `index rebuild` / `sources rebuild-index` | Rare I/O error only | Yes - the file is regenerated from scratch | Safe to retry freely || `log append` | Invalid `--op` or unreadable `--body-file` | Yes - single append | **Not idempotent.** If the previous run's outcome is uncertain, check the tail of `kb/log.md` before retrying | +| `index rebuild` | Rare I/O error only | No - each catalog file (the map plus one shard per collection/area) is rewritten independently, then stale shards are removed; an interruption can leave some regenerated and others not | Safe to retry freely - the plan is always recomputed from the pages currently on disk, so a re-run converges | +| `sources rebuild-index` | Rare I/O error only | Yes - the single provenance file is regenerated from scratch | Safe to retry freely | +| `log append` | Invalid `--op` or unreadable `--body-file` | Yes - single append | **Not idempotent.** If the previous run's outcome is uncertain, check the tail of `kb/log.md` before retrying | | `log status` | Never fails (reports 0 if `kb/log.md` is missing or empty) | Read-only | Safe to retry freely | | `lint` | Only with `--fail-on-error`: hard findings exist | Writes one report file (single atomic write) unless `--json` | Safe to retry freely, but re-run it to re-*measure*, never to re-read: the printed path holds the full report. Exit 1 means "act on the findings", not "the tool is broken" | | `search` | `rg` is not installed or did not finish within 30 s, a malformed `--field` predicate, an unknown field name, or an unknown `--backend` | Read-only | Fix the argument and retry. A timeout is a pathological pattern or an unresponsive corpus directory, not a slow answer - narrow the query or drop `--regex` rather than retrying it unchanged. An unknown field name is reported with the list of fields that do exist - it is never answered with an empty result, because that would read as "no such pages" | @@ -214,31 +216,39 @@ is atomic, and whether a retry is safe. | `publish` | git failed, **or** `--yes`/`-y` was passed. **Exit 42, not 1**, when the Mass-Update Gate, the rebase-review gate (raised by the same reconcile `sync` performs), or the Publish-Remote Gate refuses | No - sequential git operations, but both gates run before staging | For git failures: **do not retry, do not force** - report and ask the user (the reconcile step already retried the push once on its own, if a rebase resolved the rejection). For exit 42: show the user the command's full output verbatim and stop; it names the evidence and the `--confirm ` or `--confirm-rebase ` line to re-run, and re-running without it exits 42 again. The Publish-Remote Gate is the exception with no such line: it names the push URL that would have been written to and the ones this checkout allows, and only the user resolves it | | `work new` | Neither or both of `--input`/`--key` given, `--input` outside `raw/`, a `--key` that is empty or starts with `ingest-`, or the workshop already exists | Yes - one directory with two files | A collision is not transient: resume the existing run instead, or pass `--again` if the tree itself changed. Never create a numbered variant by hand | | `work close` | Unknown run key, or `--yes` was not passed | No - a recursive delete | For "not confirmed": check the listed files are no longer needed, confirm the conclusions are in `kb/`, then re-run with `--yes` | -| `sources coverage` / `sources trace` | Bad arguments (e.g. neither or both of `--raw`/`--page`) | Read-only | Fix the argument and retry | +| `sources coverage` | Never fails | Read-only | Safe to retry freely | +| `sources trace` | Neither or both of `--raw`/`--page` given, `--raw` names a file no source page covers (reported as a plain finding plus exit 1, not the usual `ERROR`-prefixed rejection), or `--page` names an unknown page | Read-only | Fix the argument and retry | | `raw accept` | A file does not exist, is not under `incoming/`, or is nested more than one level below it, two files in one call share a filename, a target path already exists, `--fidelity`/`--authority` is missing (unless `--replaces`) or names `unknown` or a value outside the schema's enum, the target name is already occupied anywhere under `raw/` by something the call does not own, `--page` names an unknown page or one with no `raw_files:` yet, an existing `raw_files:` entry is missing on disk, a file to be moved has more than one owning page, or `--page` would overwrite an already-set `fidelity`/`authority` with a different value | No - one filesystem move per file, then (with `--page`) one page write | Fix the named argument and retry once. Safe to retry as-is once the cause is fixed: a file already at its computed destination is what "already exists" reports, not a partial prior run to resume. A stem-occupied refusal is not fixed by retrying at all - it names `--replaces` and renaming in `incoming/` as the two routes and neither is the tool's to pick. Never choose the destination by hand instead - that is the decision this command exists to take away | | `raw accept --replaces` | More than one incoming file, `--page` also given, the incoming file does not exist or is not under `incoming/` (or is nested more than one level below it), its filename differs from the target's, the target does not lie under `raw/` or does not exist, `--fidelity`/`--authority` names `unknown` or a value outside the schema's enum, or the target has more than one owning source page | No - one `unlink()` + one `rename()`, plus (if `--fidelity`/`--authority` was given) one page write | Fix the named argument and retry once. Every check runs before the filesystem is touched, so a refusal leaves both files exactly as they were | -| `upload list` / `upload show` | `show`: unknown or malformed submission id | Read-only | Fix the id (see `upload list`) and retry | +| `upload list` | Never fails - a submission directory with a corrupt manifest is silently skipped | Read-only | Safe to retry freely | +| `upload show` | Unknown or malformed submission id | Read-only | Fix the id (see `upload list`) and retry | | `upload accept` | Unknown or malformed submission id, the submission's file is missing from `mcp-upload//`, or `incoming/` already exists. **Exit 42, not 1**, when `--confirm` is absent or does not match the manifest's current token - the Upload Review Gate, not a validation error | No - one filesystem move, one directory delete, one ledger append; the gate check runs first, before any of them | For exit 42: show the user the full manifest and the exact `--confirm ` re-run line printed, and stop - the same rule as every other exit-42 gate (AGENTS.md invariant 6). For the three exit-1 cases: fix the named argument and retry once; an occupied `incoming/` is not fixed by retrying unchanged - rename or clear it first | | `upload reject` | Unknown or malformed submission id, or an empty `--reason` | No - one ledger append, then one recursive delete; the ledger write happens first, so an interruption still leaves the reason on record | Fix the argument and retry once. Not idempotent against a second call with the same id: the first call already deleted the submission, so a retry reports "unknown id" - that is confirmation, not a failure | -| `types list` / `types describe` | Unknown type name | Read-only | Fix the name and retry | -| `instructions sync` / `verify` / `list` | Nothing found under `instructions/`, 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 or a skill and so risks running implicitly), something under `instructions/dev/` referenced from outside it and outside a dist:strip block, or (sync) a target directory that is not a published skill and `--force` was not passed | `sync` rewrites one directory per target (idempotent); `verify`/`list` are read-only | Fix the flagged file, then re-run. For drift, re-run `sync`: the source under `instructions/` always wins, and a published copy is never edited directly | +| `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 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 `` block. For a table of contents: run `docs toc --apply` - never hand-write the region | | `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 | | `dist export` | Target exists and is not empty, is not a directory, or the tree has no readable `VERSION` | Yes - nothing is written until every file is planned | Point `` at an empty (or new) directory and retry. Never merge into a non-empty one by hand | | `dist upgrade` | Missing local `VERSION`/`.wikitool-release.json`(`files`)/`.wikitool-kb.json`, a migration already outstanding against the installed machinery, a dirty working tree, a source with no `VERSION`/stamp/`files` block, a source version that is older than, equal to, or (without `--pre`) a pre-release relative to the installed one, or one or more locally changed files without `--keep-local` | **Yes for the refusal cases above - nothing is written.** Once writing starts it is a plain sequential file copy with no partial-state cleanup: an interruption mid-copy (killed process, disk full) can leave the tree part-old, part-new | For every refusal above: fix the named precondition and retry - none of them are transient. For locally changed files: reconcile them by hand and retry, or re-run with `--keep-local` to proceed and leave them untouched (repeatable - it reports the same files again on every subsequent run until they stop diverging). An interrupted write is not resumed automatically; compare the tree against the printed classification and finish or revert by hand | -| `version show` / `version notes` | `VERSION` is missing or unparseable; for `notes`, no `CHANGES.md` entry names the version asked for | Read-only | Fix `VERSION`, or write the changelog entry (`version bump` writes its heading). Safe to retry | +| `version show` | `VERSION` is missing or unparseable | Read-only | Fix `VERSION` and retry | +| `version notes` | An unparseable `--version`, an unreadable `VERSION` when `--version` is omitted, a missing `CHANGES.md`, or no entry naming the requested version | Read-only | Fix the named argument or file, then retry. Safe to retry | | `version check` | The feed could not be reached, answered non-JSON, or carried no `tag_name`. **Never** answers "up to date" for a question it could not ask | Read-only, no local writes | A network failure is transient - retry once, then report it. HTTP 401/403 names `$WIKITOOL_UPDATE_TOKEN`; 404 means no release exists yet or the URL points at the wrong repo | | `version bump` | More or fewer than one of `--major/--minor/--patch`, an empty `--title`, a missing `VERSION`/`CHANGES.md`, `VERSION` and the changelog's newest entry naming different versions, an escalation to a boundary crossing without `--breaking` or with neither a migration document nor `--no-migration`, `--breaking`/`--no-migration` on a bump that crosses nothing, or `--migration-required` combined with `--no-migration`, on a bump with no running candidate, with no `--no-migration` line to retract, or without a migration document already targeting the new base | No - `VERSION` then `CHANGES.md` | **Not idempotent**: a second run escalates or continues the candidate again. If the outcome is uncertain, read `VERSION` and the top of `CHANGES.md` before retrying | | `version release` | A missing `VERSION`/`CHANGES.md`, `VERSION` already a release (no running candidate), or `VERSION` and the changelog's newest entry naming different versions | No - `VERSION` then `CHANGES.md` | **Not idempotent**: a second run fails outright once the suffix is gone. If the outcome is uncertain, read `VERSION` before retrying - a release-shaped `VERSION` means it already ran | | `links show` | Page not found | Read-only | Check the exact title with `search`; a wikilink target is not always the page's stem | -| `migrate list` / `migrate status` | `list` never fails; `status` exits 1 when `.wikitool-kb.json` is missing or unreadable, or `VERSION` is | Read-only | For a missing declaration: run `migrate baseline ` once, then retry. Safe to retry freely otherwise | +| `migrate list` | Never fails | Read-only | Safe to retry freely | +| `migrate status` | `.wikitool-kb.json` is missing (content version undeclared), or `VERSION` is unreadable | Read-only | For a missing declaration: run `migrate baseline ` once, then retry. Safe to retry freely otherwise | | `migrate verify` | Only with `--fail-on-error`: an invariant changed. Also exits 1 if `--from` is not a revision in this repository | Read-only | Exit 1 from `--fail-on-error` means "act on the findings", not "the tool is broken". A finding is never fixed by re-running - it names a page and what changed on it | | `migrate done` | Unknown version, no `.wikitool-kb.json`, nothing outstanding, or a *required* version that is not the next link in the chain | Yes - single file write | **Not idempotent** for a required migration: it advances the chain. For "not the next link", run `migrate status` and apply them in the order it prints - never force the order. Recording an `offered` migration *is* idempotent and safe to repeat | | `migrate baseline` | Unparseable version, or a declaration already exists and `--force` was not passed | Yes - single file write | Safe to re-run with the same version. If a declaration exists, it is almost always `migrate done` that was wanted | | `upstream merge` | Dirty working tree, a merge already in progress, the remote does not resolve, git refused to open the merge at all (unrelated histories), or a real conflict remains in `tools/`/`types/`/`instructions/` after the content stages and stack-owned paths were restored | **No** - can leave an open, uncommitted merge behind on refusal after fetching | **Not idempotent, and not safe to retry unchanged.** For a dirty tree or an in-progress merge: fix the named precondition and retry once. For a real conflict: **do not retry, do not force** - resolve the named paths by hand (take the upstream side, or re-file the local change as an issue against the public repo per `instructions/private-instance.md`) and either `git commit --no-edit` yourself or `git merge --abort`. If the postcheck after commit finds a leak, the merge commit already exists and is **not** rolled back automatically - inspect it by hand; this is a bug report, not a retry | | `upstream verify` | A leak was found (content changed under a content stage through a path that is not stack-owned), or `--since`/`--until` is not a revision in this repository | Read-only | A finding is not fixed by re-running - it names the paths that leaked. Fix the revision argument and retry for the second case | | `doctor` | At least one check reported `FAIL` (a `WARN`, e.g. no remote or no `WIKITOOL_SESSION_ID`, does not exit 1) | Read-only | Each finding names its own fix command; re-run after applying it | -| `budget status` / `budget reset` | `reset` without `--yes`; `status` never fails | Read/rewrite of one JSON file | `status` is safe to retry. For `reset`: get the user's approval, then re-run with `--yes` | +| `budget status` | Never fails | Read-only | Safe to retry freely | +| `budget reset` | `--yes` not passed | Read/rewrite of one JSON file (or its deletion, with `--all`) | Get the user's approval, then re-run with `--yes` | | `eval sessions` | Never fails; an empty list is a valid answer | Read-only | - | | `eval score` | No trace exists for the named session | Read-only, apart from the files `--save`/`--markdown` write | Run `eval sessions` to see which ids exist. A session records nothing when telemetry is off - `WIKI_TRACE=0`, or a distributed instance with no opt-in (`wikitool doctor` says which) - so an absent trace is not necessarily a fault. Safe to retry | | *(any command)* - Iteration Budget Gate / Loop-Breaker | Session call limit exceeded, or the last 3 calls were identical | N/A - pre-dispatch check, the command never ran | **Not** safe to retry as-is; retrying is the failure mode being prevented. Stop and escalate | diff --git a/tools/chemenu/commands/docs_verify.py b/tools/chemenu/commands/docs_verify.py index f5b621a..0bbfece 100644 --- a/tools/chemenu/commands/docs_verify.py +++ b/tools/chemenu/commands/docs_verify.py @@ -5,7 +5,10 @@ The wiki's own rule is that a derived copy of recomputable truth must be checked or absent. Three such copies survive on purpose because they earn their keep as reading material: - 1. `tools/CONTRACT.md`'s command table (re-derivable from the Typer app) + 1. `tools/CONTRACT.md`'s two command tables - § Commands and § Error + contracts - each re-derivable from the Typer app and checked + independently, in both directions, so a row surviving in one table + cannot hide its own deletion from the other 2. the collection and stage contracts (their existence and placement, not their content) 3. the absence of pre-type-system `type: entity` frontmatter in the @@ -178,6 +181,35 @@ LEGACY_TYPE_RE = re.compile(r"^type:\s*(entity|concept|source|comparison)\s*$", # First backticked cell of a markdown table row, e.g. "| `xref add --a ...` | ... |" TABLE_CELL_RE = re.compile(r"^\|\s*`([^`]+)`", re.MULTILINE) +# tools/CONTRACT.md carries two tables whose first cell is a backticked +# command path - § Commands and § Error contracts - and `check_cli_readme` +# must not treat them as one pot (Gitea #91): a row deleted from one used to +# go unnoticed as long as the same name survived in the other, and the +# second table was not enforced against anything at all. +COMMANDS_HEADING = "## Commands" +ERROR_CONTRACTS_HEADING = "## Error contracts" + + +def section_text(full_text: str, heading: str) -> str: + """The text of one `##`-level section: from just after `heading`'s own + line up to the next `#`- or `##`-level heading, or the end of the + document. + + Raises `ValueError` if `heading` is not found verbatim, rather than + falling back to scanning the whole document - a renamed heading has to + surface as a failure, because silently widening the scope back to + "everything" is exactly the bug this function exists to prevent from + reappearing under a different name. + """ + pattern = re.compile( + r"^" + re.escape(heading) + r"[ \t]*\n(.*?)(?=^#{1,2}[ \t]|\Z)", + re.MULTILINE | re.DOTALL, + ) + match = pattern.search(full_text) + if match is None: + raise ValueError(f"no {heading!r} heading found") + return match.group(1) + def registered_commands() -> set[str]: """Every command path the CLI exposes, e.g. {'new', 'xref add', ...}. @@ -213,15 +245,23 @@ def documented_commands(readme_text: str) -> list[str]: def check_cli_readme() -> list[str]: - """Every registered command must appear in tools/CONTRACT.md, and every - command documented there must exist. + """Every registered command must appear in tools/CONTRACT.md's own + § Commands table, and separately in its § Error contracts table - each + direction checked per table, independently of the other. - `TABLE_CELL_RE` scans the whole file for any markdown table row whose - first cell is backticked - there is no separate "command table" region it - is scoped to, so a match in the error-contract table (or any other table - shaped the same way) counts too. It reads only the backticked path itself, - never the rest of the cell: a changed flag or a rewritten description in - an existing row is invisible to this check. + The two tables used to be read as one pot: `TABLE_CELL_RE` matched a + backticked first cell anywhere in the file, so a row deleted from + § Commands went unnoticed as long as the same name still had a row in + § Error contracts, and § Error contracts was never itself compared + against the registered commands (Gitea #91). `section_text` scopes each + table to the text between its own `##` heading and the next one, and + raises rather than silently scanning the whole file if a heading has been + renamed or removed - a renamed heading must be reported, not read as + "table now empty" or "table now everything". + + Within a section, only the first backticked cell of each row is read - + a changed flag or a rewritten description in an existing row is invisible + to this check, on purpose: it verifies presence, never prose. The reverse check matches a documented cell against the full registered command path (e.g. `xref add`, `migrate verify`), not just its first @@ -233,18 +273,36 @@ def check_cli_readme() -> list[str]: return [f"{CLI_README.relative_to(config.ROOT)} is missing"] text = CLI_README.read_text(encoding="utf-8") - cells = documented_commands(text) - issues = [] - registered = sorted(registered_commands()) - for command_path in registered: - if not any(cell == command_path or cell.startswith(command_path + " ") for cell in cells): - issues.append(f"command `{command_path}` is not documented in tools/CONTRACT.md") + issues: list[str] = [] - for cell in cells: - if not any(cell == cp or cell.startswith(cp + " ") for cp in registered): - first_token = cell.split(" ", 1)[0] - issues.append(f"tools/CONTRACT.md documents `{cell}`, but `{first_token}` is not a wikitool command") + for heading, label in ( + (COMMANDS_HEADING, "§ Commands"), + (ERROR_CONTRACTS_HEADING, "§ Error contracts"), + ): + try: + section = section_text(text, heading) + except ValueError as exc: + issues.append( + f"tools/CONTRACT.md: {exc} - its {label} table cannot be checked against the CLI" + ) + continue + + cells = documented_commands(section) + + for command_path in registered: + if not any(cell == command_path or cell.startswith(command_path + " ") for cell in cells): + issues.append( + f"command `{command_path}` is not documented in tools/CONTRACT.md's {label} table" + ) + + for cell in cells: + if not any(cell == cp or cell.startswith(cp + " ") for cp in registered): + first_token = cell.split(" ", 1)[0] + issues.append( + f"tools/CONTRACT.md's {label} table documents `{cell}`, but `{first_token}` " + "is not a wikitool command" + ) return issues diff --git a/tools/chemenu/tests/test_docs_verify.py b/tools/chemenu/tests/test_docs_verify.py index 7db91bf..455b2e6 100644 --- a/tools/chemenu/tests/test_docs_verify.py +++ b/tools/chemenu/tests/test_docs_verify.py @@ -42,13 +42,136 @@ def test_invented_subcommand_under_a_real_group_is_caught(tmp_path, monkeypatch) group. The reverse check must match the full registered path, not just the top-level word.""" fake = tmp_path / "README.md" - fake.write_text("| `xref frobnicate --a X --b Y` | does not exist |\n", encoding="utf-8") + fake.write_text( + "## Commands\n\n" + "| `xref frobnicate --a X --b Y` | does not exist |\n\n" + "## Error contracts\n\n" + "| `xref frobnicate --a X --b Y` | does not exist |\n", + encoding="utf-8", + ) monkeypatch.setattr(docs_verify, "CLI_README", fake) monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"xref add", "xref remove"}) issues = docs_verify.check_cli_readme() assert any("xref frobnicate" in issue for issue in issues) +# --- section-scoped § Commands vs. § Error contracts (Gitea #91) ------------ + + +def test_section_text_extracts_between_headings(): + text = "# T\n\n## A\n\nfoo\n\n## B\n\nbar\n" + assert docs_verify.section_text(text, "## A").strip() == "foo" + + +def test_section_text_extends_to_end_of_file_when_last(): + text = "# T\n\n## A\n\nfoo\nbar\n" + assert docs_verify.section_text(text, "## A").strip() == "foo\nbar" + + +def test_section_text_raises_on_missing_heading(): + with pytest.raises(ValueError): + docs_verify.section_text("# T\n\nno headings here\n", "## Commands") + + +def _fake_contract(tmp_path, commands_rows: str, error_rows: str): + fake = tmp_path / "CONTRACT.md" + fake.write_text( + f"## Commands\n\n{commands_rows}\n## Error contracts\n\n{error_rows}\n", + encoding="utf-8", + ) + return fake + + +def test_a_row_deleted_from_commands_is_caught_even_if_error_contracts_still_has_it( + tmp_path, monkeypatch +): + """Regression for the bug the section split fixes: before, a name + surviving in either table hid its own deletion from the other, so + § Commands losing a row was invisible as long as § Error contracts still + named it.""" + fake = _fake_contract( + tmp_path, + commands_rows="| Command | Purpose |\n", # `frobnicate`'s row was deleted here + error_rows="| `frobnicate` | never | yes | retry |\n", + ) + monkeypatch.setattr(docs_verify, "CLI_README", fake) + monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"}) + issues = docs_verify.check_cli_readme() + assert any( + "frobnicate" in issue and "§ Commands" in issue and "not documented" in issue + for issue in issues + ) + + +def test_error_contracts_is_enforced_against_registered_commands(tmp_path, monkeypatch): + """Before the split, § Error contracts was never itself compared against + the registered commands - a row missing there was invisible.""" + fake = _fake_contract( + tmp_path, + commands_rows="| `frobnicate` | does things |\n", + error_rows="| Command | Exit 1 means | Atomic? | Retry policy |\n", + ) + monkeypatch.setattr(docs_verify, "CLI_README", fake) + monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"}) + issues = docs_verify.check_cli_readme() + assert any( + "frobnicate" in issue and "§ Error contracts" in issue and "not documented" in issue + for issue in issues + ) + + +def test_a_phantom_error_contract_row_is_reported(tmp_path, monkeypatch): + """The reverse direction inside § Error contracts: a row for a command + that does not exist must be reported there too, not only in § Commands.""" + fake = _fake_contract( + tmp_path, + commands_rows="| `frobnicate` | does things |\n", + error_rows="| `frobnicate` | never | yes | retry |\n| `ghost command` | never happened | no | n/a |\n", + ) + monkeypatch.setattr(docs_verify, "CLI_README", fake) + monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"}) + issues = docs_verify.check_cli_readme() + assert any( + "ghost command" in issue and "§ Error contracts" in issue and "not a wikitool command" in issue + for issue in issues + ) + + +def test_a_renamed_commands_heading_is_reported_not_silently_scanned(tmp_path, monkeypatch): + """A renamed or removed `## Commands` heading must fail loudly - falling + back to scanning the whole file would make the two tables indistinguishable + again, which is the exact bug this check exists to prevent.""" + fake = tmp_path / "CONTRACT.md" + fake.write_text( + "## Kommandos\n\n" + "| `frobnicate` | does things |\n\n" + "## Error contracts\n\n" + "| `frobnicate` | never | yes | retry |\n", + encoding="utf-8", + ) + monkeypatch.setattr(docs_verify, "CLI_README", fake) + monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"}) + issues = docs_verify.check_cli_readme() + assert any("no '## Commands' heading found" in issue for issue in issues) + assert not any("§ Commands" in issue and "not documented" in issue for issue in issues) + + +def test_a_renamed_error_contracts_heading_is_reported_not_silently_scanned(tmp_path, monkeypatch): + fake = tmp_path / "CONTRACT.md" + fake.write_text( + "## Commands\n\n" + "| `frobnicate` | does things |\n\n" + "## Fehlerkontrakte\n\n" + "| `frobnicate` | never | yes | retry |\n", + encoding="utf-8", + ) + monkeypatch.setattr(docs_verify, "CLI_README", fake) + monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"}) + issues = docs_verify.check_cli_readme() + assert any("no '## Error contracts' heading found" in issue for issue in issues) + assert not any("§ Error contracts" in issue and "not documented" in issue for issue in issues) + + def test_collection_contracts_exist(): assert docs_verify.check_collection_contracts() == []