From 576df2cdddc96614a7e6641e562022d52112d411 Mon Sep 17 00:00:00 2001 From: Torben Nehmer Date: Wed, 2 Sep 2026 07:19:32 +0200 Subject: [PATCH] feat: MCP-Leseserver, Bibliotheksgrenze, Haertung des Lesepfads, Publish-Remote-Gate scharf (2.4.0) Files changed: - .gitea/workflows/ci.yml - CHANGES.md - README.md - VERSION - instructions/mcp-read-server.md - tools/CONTRACT.md - tools/README.md - tools/chemenu/api.py - tools/chemenu/commands/doctor.py - tools/chemenu/commands/lint.py - tools/chemenu/commands/search.py - tools/chemenu/commands/types_cmd.py - tools/chemenu/config.py - tools/chemenu/corpus_cache.py - tools/chemenu/errors.py - tools/chemenu/frontmatter_io.py - tools/chemenu/lint_core.py - tools/chemenu/mcp/__init__.py - tools/chemenu/mcp/__main__.py - tools/chemenu/mcp/server.py - tools/chemenu/page.py - tools/chemenu/search/filters.py - tools/chemenu/search/registry.py - tools/chemenu/search/ripgrep.py - tools/chemenu/search/service.py - tools/chemenu/tests/conftest.py - tools/chemenu/tests/test_api.py - tools/chemenu/tests/test_corpus_cache.py - tools/chemenu/tests/test_doctor.py - tools/chemenu/tests/test_frontmatter_io.py - tools/chemenu/tests/test_instructions_cmd.py - tools/chemenu/tests/test_mcp_server.py - tools/chemenu/tests/test_new_page.py - tools/chemenu/tests/test_search.py - tools/chemenu/type_resolver.py - tools/chemenu/types_core.py - tools/requirements-mcp.txt --- .gitea/workflows/ci.yml | 4 + CHANGES.md | 244 ++++++++++ README.md | 25 ++ VERSION | 2 +- instructions/mcp-read-server.md | 108 +++++ tools/CONTRACT.md | 34 +- tools/README.md | 26 +- tools/chemenu/api.py | 202 +++++++++ tools/chemenu/commands/doctor.py | 15 +- tools/chemenu/commands/lint.py | 444 ++----------------- tools/chemenu/commands/search.py | 101 ++--- tools/chemenu/commands/types_cmd.py | 81 ++-- tools/chemenu/config.py | 170 ++++++- tools/chemenu/corpus_cache.py | 123 +++++ tools/chemenu/errors.py | 31 ++ tools/chemenu/frontmatter_io.py | 153 +++++-- tools/chemenu/lint_core.py | 431 ++++++++++++++++++ tools/chemenu/mcp/__init__.py | 8 + tools/chemenu/mcp/__main__.py | 79 ++++ tools/chemenu/mcp/server.py | 234 ++++++++++ tools/chemenu/page.py | 8 + tools/chemenu/search/filters.py | 3 +- tools/chemenu/search/registry.py | 21 +- tools/chemenu/search/ripgrep.py | 60 ++- tools/chemenu/search/service.py | 112 +++++ tools/chemenu/tests/conftest.py | 34 +- tools/chemenu/tests/test_api.py | 184 ++++++++ tools/chemenu/tests/test_corpus_cache.py | 98 ++++ tools/chemenu/tests/test_doctor.py | 34 ++ tools/chemenu/tests/test_frontmatter_io.py | 103 ++++- tools/chemenu/tests/test_instructions_cmd.py | 5 + tools/chemenu/tests/test_mcp_server.py | 274 ++++++++++++ tools/chemenu/tests/test_new_page.py | 3 + tools/chemenu/tests/test_search.py | 95 +++- tools/chemenu/type_resolver.py | 10 +- tools/chemenu/types_core.py | 90 ++++ tools/requirements-mcp.txt | 13 + 37 files changed, 3037 insertions(+), 625 deletions(-) create mode 100644 instructions/mcp-read-server.md create mode 100644 tools/chemenu/api.py create mode 100644 tools/chemenu/corpus_cache.py create mode 100644 tools/chemenu/errors.py create mode 100644 tools/chemenu/lint_core.py create mode 100644 tools/chemenu/mcp/__init__.py create mode 100644 tools/chemenu/mcp/__main__.py create mode 100644 tools/chemenu/mcp/server.py create mode 100644 tools/chemenu/search/service.py create mode 100644 tools/chemenu/tests/test_api.py create mode 100644 tools/chemenu/tests/test_corpus_cache.py create mode 100644 tools/chemenu/tests/test_mcp_server.py create mode 100644 tools/chemenu/types_core.py create mode 100644 tools/requirements-mcp.txt diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index f5a32ed..7507ee0 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -101,6 +101,10 @@ jobs: # instance does not measure this suite. Installed beside pytest for # the same reason pytest itself is. tools/.venv/bin/pip install --quiet pytest pytest-cov + # The MCP server's dependency is optional for an instance but not for + # CI: its tests skip without it, and a skipped golden test is exactly + # how the server's output and the CLI's would drift apart unnoticed. + tools/.venv/bin/pip install --quiet -r tools/requirements-mcp.txt - name: Tests # Not run with WIKI_TRACE=0: two telemetry tests assert that a trace is diff --git a/CHANGES.md b/CHANGES.md index 8025f9b..7187d41 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -20,6 +20,250 @@ their date-only headings. --- +## 2.4.0 - 2026-09-02 - MCP-Leseserver: zweiter Konsument auf demselben Kern + +**Author:** Torben Nehmer + +Letzter Schritt der Sequenz aus #36, inhaltlich Issue #19. `chemenu` bekommt einen zweiten +Konsumenten: `search`, `types`, `describe_type`, `lint` und `status` über MCP. Kein CLI mit +angeschraubter Netzwerkschnittstelle — CLI und Server sind zwei Adapter auf dem Kern, den 2.3.0 +freigelegt hat. + +**`tools/chemenu/mcp/`**, im Repo statt als eigenes Artefakt. Der Golden-Test, der die +Serverantworten gegen die `--json`-Formen der CLI hält, läuft nur mit beiden Seiten in einer +Testsuite; getrennt würde aus einem Contract eine Versionsabsprache. Der Test ruft `wikitool` als +Subprozess gegen denselben Baum auf, über `CHEMENU_ROOT` — womit er nebenbei die Root-Auflösung +von außen mitprüft. + +**Zwei Transports.** `stdio` zum Entwickeln und Testen ohne Netz, `streamable-http` für die +Auslieferung — der einzige, vor den sich die Authentifizierungs-Middleware überhaupt setzen kann, +weil sie ein HTTP-Reverse-Proxy ist. `sse` ist über das SDK erreichbar und wird bewusst nicht +angeboten: der abgelöste Remote-Transport, jetzt darauf zu bauen verschiebt den Wechsel nur. +`--host`/`--port` gibt es, weil der Default auf Loopback bindet und ein Container hinter einem +Proxy eine Adresse braucht, die der Proxy erreicht — eine Eigenschaft der Software, nicht einer +Installation. Beide Transports sind gegen den echten Korpus gegengeprüft. + +**Kein Schreibpfad, strukturell.** Weder der Server noch `chemenu.api` importiert irgendetwas +unter `chemenu.commands`, also existieren `new`, `touch`, `xref`, `cite`, `publish`, `migrate` +und `version bump` in dieser Reichweite gar nicht, statt aus einer Liste gefiltert zu werden. Ein +Test importiert das Servermodul in einem frischen Interpreter und sieht in `sys.modules` nach; +ein zweiter ruft alle fünf Tools auf und vergleicht den Dateibaum, `HEAD` und +`git status --porcelain` vorher/nachher. + +**Jede Antwort trägt ihren Commit.** `commit` und `as_of` in jedem Payload; `null` heißt, der +bediente Baum hat uncommittete Änderungen und die Antwort entspricht keiner Revision. Der Stempel +ist die Revision, aus der die Seiten *tatsächlich* gelesen wurden — zwischen Laden und Stempeln +kann der Baum sich bewegen, deshalb reicht der Ladepfad seine Revision durch, statt noch einmal +zu fragen. Das war beim ersten Durchlauf falsch: `types`/`lint`/`status` lasen die zuletzt +*gecachte* Revision und stempelten `null`, obwohl der Baum sauber war. + +**Telemetrie in den bedienten Baum wird beim Start verweigert**, nicht stillschweigend +umgeleitet. Tracing ist per Default an und schreibt nach `reports/telemetry/` im Repo — genau das +Verzeichnis, das der Sync per `git reset --hard` wegräumen darf. `WIKI_TRACE=0` oder +`WIKI_TRACE_DIR` außerhalb des Korpus. Heute schreibt auf diesem Pfad nichts (der Emitter hängt an +`cli.main()` und den Gates), die Sperre ist gegen später. + +**Fehler an der Protokollgrenze.** Ein `ChemenuError` wird zum `ToolError` des SDK — eine +absichtliche Ablehnung, deren Text den Aufrufer erreicht. Alles andere bleibt ein Absturz, dessen +Text auf dem Server bleibt. Ein kaputtes Prädikat ist das Argument des Aufrufers, also muss die +Zeile mitreisen, die sagt, was stattdessen zu schreiben ist. + +**Bewusst nicht enthalten:** Authentifizierung und Rate Limiting (Middleware vor dem Prozess), +Deployment (private Infrastruktur), der Iteration Budget Gate — er begrenzt eine Agenten-Session +und nicht einen Nutzer, weshalb Retrieval von ihm befreit ist; ihn hier als Rate Limiter zu +benutzen würde ihn dazu verwässern. + +**Die Abhängigkeit ist optional** (`tools/requirements-mcp.txt`): eine Instanz, die nur die CLI +benutzt, soll dafür nicht pydantic, starlette, uvicorn und cryptography installieren müssen. CI +installiert sie, denn ein übersprungener Golden-Test ist genau der Weg, auf dem Server und CLI +unbemerkt auseinanderlaufen. + +Betrieb und Sync-Mechanismus: [instructions/mcp-read-server.md](instructions/mcp-read-server.md). +Polling (`git fetch && git reset --hard`) statt Webhook — kein eingehender Endpunkt, keine +Signaturprüfung. `reset --hard` ist dort tragend und keine Bequemlichkeit: ein abgedrifteter Baum +antwortet zwar richtig, parst aber bei jeder Anfrage neu und stempelt jede Antwort mit `null`. + +**Dateien:** `chemenu/mcp/` (neu: `server.py`, `__main__.py`), `chemenu/api.py`, +`tools/requirements-mcp.txt` (neu), `instructions/mcp-read-server.md` (neu), `tools/CONTRACT.md`, +`tools/README.md`, `.gitea/workflows/ci.yml`, `tests/test_mcp_server.py` (neu). + +--- + +## 2.3.0 - 2026-09-02 - Bibliotheksgrenze: chemenu laesst sich auf einen Korpus zeigen + +**Author:** Torben Nehmer + +Dritter Schritt der Sequenz aus #36, inhaltlich Issue #31. Der Schritt, der `chemenu` von einem +Skript, das in seinem eigenen Verzeichnis lebt, zu einer Bibliothek macht, auf die man einen +Korpus *zeigen* kann. Rückwärtskompatibel: ohne gesetzte Variable verhält sich `tools/wikitool` +unverändert. + +**Root-Auflösung nach Präzedenz statt nach Dateilage.** `config.resolve_root()`: expliziter +Parameter → `$CHEMENU_ROOT` → Walk-up wie bisher. Der Walk-up bleibt Default. Wichtiger als die +neue Fähigkeit ist die beseitigte Fehlerklasse: `ROOT` und alles darunter waren +Modulkonstanten, also **zur Importzeit gebunden**, und +`monkeypatch.setattr(config, "ROOT", ziel)` zeigte `ROOT` um, aber nicht `KB_DIR`/`RAW_DIR`. Wer +sich darauf verließ, baute etwas, das scheinbar auf einem Zielbaum arbeitete und in Wahrheit aus +dem Entwickler-Checkout antwortete. Die abgeleiteten Pfade werden jetzt bei jedem Zugriff +aufgelöst (PEP 562 `__getattr__`) und folgen `ROOT` — der halb-umgezeigte Zustand ist nicht mehr +konstruierbar. `CHEMENU_ROOT` ist in `_WIKITOOL_ENV` registriert, #23-konform. + +**`config.reset()` und `config.rooted()`.** `reset()` nimmt Zuweisungen auf die verwalteten +Pfadnamen zurück; die Testsuite ruft es zwischen Tests, und das ist dort nicht optional: +`monkeypatch` merkt sich den alten Wert, indem es ihn *liest* — also auflöst — und schreibt ihn +beim Aufräumen als echtes Attribut zurück. Genau die stale Bindung, die der Umbau unmöglich +machen sollte, vom Cleanup wieder aufgebaut. `rooted(root)` setzt den Root für die Dauer eines +Blocks, prozessweit und damit nicht thread-sicher — der Aufrufer hält das Lock, dieselbe +Disziplin wie beim Korpus-Cache. Nötig, weil nicht alles einen Root als Argument nimmt: der +modulglobale `TypeResolver` muss `types/` finden, und ohne ihn läse ein fremder Korpus mit den +Type-Specs *dieses* Checkouts. + +**Die Naht ist gezogen.** `run_search`/`run_lint`/`types` lagen in Modulen, die `typer` auf +Modulebene importieren und über `_util` auch `rich` — wer sie in-process aufrief, zog den +kompletten CLI-Kopf mit. Der reine Kern liegt jetzt in `search/service.py`, `lint_core.py` und +`types_core.py`; `commands/` sind die Terminal-Adapter darüber und re-exportieren die Namen, damit +kein bestehender Import bricht. Ein Test importiert `chemenu.api` in einem frischen Interpreter +und prüft, dass weder `chemenu.commands.*` noch `typer`/`rich`/`click` geladen werden. + +**`chemenu.api.Corpus` als In-Process-Einstieg.** Nimmt einen Root, liefert exakt die +`--json`-Formen der CLI zurück — ein Wire-Contract statt zwei — und stempelt jede Antwort mit dem +Commit-SHA und einem Zeitstempel (`commit`, `as_of`), so dass aus einer stillen veralteten Antwort +eine sichtbare wird. `search`/`lint`/`types`/`describe_type`/`status`; `status` ist bewusst +serverseitig **komponiert** und kein Wrapper, weil es kein `wikitool status` gibt. Lesend +strukturell: nichts unter `chemenu.commands` wird importiert, die Schreibfunktionen existieren in +dieser Oberfläche also gar nicht, statt gefiltert zu werden. Das ist die Grenze, auf der #19 +aufsetzt. + +**Exceptions statt Exit-Codes an der Grenze.** `chemenu/errors.py`: `ChemenuError` mit +`ValidationError` (Eingabe abgelehnt) und `BackendError` (Abhängigkeit fehlt oder scheitert). +`PredicateError`, `FrontmatterError`, `UnknownBackend` und die beiden `Ripgrep*` hängen jetzt +darunter; `ValidationError` erbt zusätzlich von `ValueError`, weil `PredicateError` vorher eines +war und Aufrufer es so fangen. Das CLI-Verhalten ist unverändert: `fail()` → `ERROR`-Zeile, +Exit 1, Budget-Refund. + +**`resolve()` reicht den Root an das Backend durch.** Vorher konnte ein Aufrufer `run_search` einen +Korpus übergeben, während `RipgrepBackend` weiter `config.KB_DIR` durchlief — die Anfrage aus dem +einen Baum beantwortet, die Seiten aus dem anderen gelesen, ohne dass irgendetwas das gesagt +hätte. + +**Zwei Abhängigkeiten, die durch Zufall hielten, stehen jetzt da.** `TypeResolver.repo_root` +folgt `ROOT`, statt beim Import zu binden — womit Fixtures, die `ROOT` auf einen tmp-Baum zeigen, +die mitgelieferten Type-Specs ausdrücklich benennen müssen (`use_shipped_type_specs`). Dieselbe +Form wie das Loch, für das `raw_dir` geschrieben wurde, eine Ebene tiefer. Und +`types describe --json` trug `root:` nicht im Payload, obwohl `types list --json` es tut: +`types/instruction.md` deklariert `root: repo`, die Renderer-Zeile las es direkt aus dem +Frontmatter. Jetzt im Payload, in derselben Form wie bei `list`. + +**Dateien:** `config.py`, `errors.py` (neu), `api.py` (neu), `search/service.py` (neu), +`lint_core.py` (neu), `types_core.py` (neu), `search/registry.py`, `type_resolver.py`, +`commands/search.py`, `commands/lint.py`, `commands/types_cmd.py`, `tools/CONTRACT.md`, dazu +`tests/conftest.py`, `tests/test_api.py` (neu), `tests/test_new_page.py`, +`tests/test_instructions_cmd.py`. + +--- + +## 2.2.4 - 2026-09-02 - Haertung des Lesepfads: ReDoS, Subprozess-Timeout, YAML-Alias-Budget, Korpus-Cache + +**Author:** Torben Nehmer + +Zweiter Schritt der Sequenz aus #36, inhaltlich Issue #33. Limits vor der Exposition: der +Lesepfad bekommt mit dem MCP-Server (#19) einen Konsumenten, der nicht der Operator ist. Alle +sechs Befunde waren reproduziert, nicht vermutet; jeder hat jetzt eine Regression. + +**ReDoS über `--regex` beseitigt.** `search/ripgrep.py:_contains` gab nutzergesteuerten Regex an +Pythons Backtracking-Engine: `(\w+\s?)+$` gegen 114 Zeichen gewöhnlichen Seiteninhalts terminiert +nicht in acht Sekunden, ein deterministisch scheiterndes Muster braucht 0,2 ms — der Unterschied +ist das Muster, nicht der Heuhaufen. `build_hit` ruft die Funktion je Treffer zweimal auf, und +`\w` matcht jede Seite: eine Anfrage kaufte zwei unbegrenzte Suchen pro Korpusseite. Der Zweig ist +**gelöscht**, nicht begrenzt. `rg` hat das Muster mit einer linearen Engine längst angewendet, wenn +die Funktion überhaupt läuft; verloren geht nur der zusätzliche Titel-/Summary-Bonus für +nicht-literale Muster, und Summary wie H1 sind selbst Zeilen in der Datei, die `rg` zählt. + +**Subprozess-Timeout.** `rg` wird nach 30 s abgeräumt und über den vorhandenen +`RipgrepFailed`-Pfad gemeldet. Kein Performance-Budget — eine Fixed-String-Suche kostet hier 6 ms — +sondern ein Hänger-Abbruch, damit ein Aufruf als Fehler endet statt den Aufrufer offenzuhalten, +während seine Ausgabe in den Heap puffert. + +**YAML-Anchors und -Aliases im Frontmatter werden verweigert, nicht budgetiert.** Gemessen: +267 Byte werden in 0,2 ms zu 672.603 Knoten beim Traversal, Wachstum 9ⁿ bei konstanter Parse-Zeit +— ein Größenlimit fasst das nicht an, weil die Eingabe klein bleibt. Die Prüfung läuft auf dem +*Event*-Strom (`yaml.parse`), der nichts auflöst, kostet also O(Text) und löst nie aus, wonach sie +fragt; `*` ist in jedem Alias-Knoten notwendig, seine Abwesenheit beweist Abwesenheit ohne jeden +Parse — der Weg, den jede echte Seite nimmt. Dazu ein Größenlimit von 64 KiB und ein Abfangen von +`RecursionError` (PyYAML komponiert rekursiv, tiefe Verschachtelung ist kein `YAMLError`). Heute +nicht erreichbar, weil `kb/` der Operator committet; erreichbar mit der Ingest-Queue (#32). + +**`CSafeLoader` statt `SafeLoader`, mit Fallback.** Gemessen über diesen Korpus (176 Seiten, +best of 5): **265 ms → 54 ms**. Kein Mikro-Tuning — der Korpus-Parse war der größte Einzelposten +eines `search`-Aufrufs und skaliert linear mit der Korpusgröße. End-to-end fällt ein +`wikitool search` damit von 593 ms auf **347 ms**; die verbleibenden 262 ms sind Modulimport und +entfallen erst im residenten Prozess (#19). + +**Stiller Frontmatter-Verlust wird gemeldet.** Entschieden: der Lesepfad *nennt* die Seite, statt +sie zu schlucken. Kaputtes YAML wird weiterhin zu `{}` — Massenoperationen dürfen an einer Seite +nicht scheitern —, aber der Grund wird mitgeführt (`Page.frontmatter_error`) und ausgegeben: +`search --json` trägt immer eine `unreadable`-Liste aus `{path, reason}`, die Tabellenform +schreibt dieselben Zeilen nach stderr. Das war nötig, weil so eine Seite weder `confidence` noch +`kind` hat und damit aus jedem positiven `--field`-Prädikat fällt — ausgerechnet aus der +Low-Confidence-Suche, die Seiten in genau diesem Zustand finden soll — und dabei aussieht wie eine +Seite, die nicht gematcht hat. Für Frontmatter, das diese Instanz nicht selbst geschrieben hat, +steht `read_page_strict()` bereit: die Quarantäne aus #32 muss strikt lesen, wo ein +unlesbares Dokument das Dokument stoppen und nicht leeren soll. + +**Ein Parser statt zwei.** `read_page()` und `frontmatter_error()` liefen bisher getrennt durch +`safe_load` — so konnte der permissive Weg zu `{}` degradieren aus einem Grund, den der strikte +Weg anders beschrieb, und jeder Aufrufer, der beide Antworten wollte, las die Datei zweimal. +Beide gehen jetzt durch `_load_frontmatter()`; ein Test hält sie gegeneinander. + +**Korpus-Cache am Commit-SHA** (`chemenu/corpus_cache.py`), als Objekt, das ein Aufrufer *hält* — +kein Modul-Dict, das sich hinter allen einschaltet. Die CLI hält keins und verhält sich unverändert +(ein Aufruf pro Prozess, nichts wiederzuverwenden); der residente Prozess aus #19 hält eins. +Entscheidend ist nicht die Geschwindigkeit, sondern dass nichts veraltet: **ein schmutziger +Arbeitsbaum wird nie gecacht**, sonst bekäme eine Sitzung, die eine Seite schreibt und danach +sucht, die Analyse von vor dem Schreiben — bei unverändertem SHA. Kann git nicht antworten, gilt +der Baum als schmutzig. Derselbe SHA ist der Antwort-Stempel aus #19, per Konstruktion also die +Revision, aus der die Antwort auch wirklich berechnet wurde. + +**Erhalten geblieben** ist die Eigenschaft, die kein Befund war: kein `shell=True`, +`--fixed-strings` als Default, `--`-Terminator. Der Modul-Docstring führt sie jetzt als drei +tragende Zusagen statt zwei — die dritte ist, dass nutzergesteuerte Muster ausschließlich `rg` +sieht. + +**Dateien:** `frontmatter_io.py`, `search/ripgrep.py`, `commands/search.py`, `page.py`, +`corpus_cache.py` (neu), `tools/CONTRACT.md`, dazu `tests/test_frontmatter_io.py`, +`tests/test_search.py`, `tests/test_corpus_cache.py` (neu). + +--- + +## 2.2.3 - 2026-09-02 - Publish-Remote-Gate in diesem Checkout scharf, doctor benennt den Zustand + +**Author:** Torben Nehmer + +Erster Schritt der Sequenz aus Issue #36 (Weg zum MCP-Leseserver), inhaltlich Issue #34. Das +Gate war seit 2.2.0 implementiert und in diesem Checkout **inert**: `.wikitool-remotes.json` +fehlte, und eine fehlende Datei heißt unbeschränkt. `ENVIRONMENT.md` beschrieb `origin` +gleichzeitig als einziges Publish-Ziel, `AGENTS.md` führt das Gate unter den drei in Code +erzwungenen Grenzen. Eine dokumentierte Sicherung, die nicht greift, ist schlechter als eine +fehlende — sie erzeugt genau das Vertrauen, das sie nicht verdient. + +**Die Datei ist angelegt** (gitignored, per Checkout, reist nicht mit) und gegengeprüft: ein +`publish --remote` auf ein nicht gelistetes Ziel verweigert mit Exit 42, bevor irgendetwas +gestaged wird, und der Arbeitsbaum bleibt unberührt. Damit steht die Sicherung **vor** dem Klonen +der privaten Instanz (#30) — nachträglich gesetzt ließe sie genau das Fenster offen, das sie +schließt. + +**`doctor` sagt jetzt, ob das Gate scharf ist, nicht nur ob die Datei da ist.** Alle drei +Zustände beginnen mit `Gate armed:` bzw. `Gate not armed:`; der einzelne Remote ohne Allowlist +bleibt `OK` (er hat nichts zu schützen, und ein FAIL machte die Datei durch die Hintertür +verpflichtend), sagt aber ausdrücklich, dass jedes Push-Ziel durchkommt. Der Fall, der wirklich +beißt — mehrere Remotes ohne Allowlist — bleibt `WARN`. Der Check hatte bislang **keine Tests**; +drei sind dazugekommen, einer je Zustand. + +**Dateien:** `.wikitool-remotes.json` (neu, nicht committet), `doctor.check_publish_remotes()`, +`tools/chemenu/tests/test_doctor.py`. + +--- + ## 2.2.2 - 2026-09-01 - Doku-Verdrahtung: Publish-Remote Gate im Werkzeugvertrag, Projektseite auf oeffentlich **Author:** Torben Nehmer diff --git a/README.md b/README.md index f7e431a..60e42b2 100644 --- a/README.md +++ b/README.md @@ -342,6 +342,31 @@ under `instructions/dev/` (never present in a distributed instance - `tools/CONT explains why). +### MCP read server (optional) + +The terminal is not the only way in. `tools/chemenu/mcp/` serves the same wiki read-only over +MCP - `search`, `types`, `describe_type`, `lint` and `status` - so a consumer that is not a +shell on this machine can ask the same questions and get the same answers. Literally the same: +the CLI and the server are two adapters over one core, and a golden test holds their output +together rather than trusting that it agrees. + +There is no tool that writes, and not because one is filtered out of a list: the server imports +nothing under `chemenu/commands/`, so `new`, `publish` and the rest are unreachable from it. +Every answer carries the commit it was computed from, so a checkout that has fallen behind +produces a visibly stale answer instead of a confident wrong one. + +```bash +tools/.venv/bin/pip install -r tools/requirements-mcp.txt +WIKI_TRACE=0 tools/.venv/bin/python -m chemenu.mcp # stdio +WIKI_TRACE=0 tools/.venv/bin/python -m chemenu.mcp \ + --transport streamable-http --host 0.0.0.0 --port 8000 # deployed +``` + +The dependency is deliberately not in `requirements.txt`: an instance that only uses the CLI +should not have to install a web stack to do it. Running it, keeping its checkout current, and +where authentication belongs (in front of the process, not in it) are in +[`instructions/mcp-read-server.md`](instructions/mcp-read-server.md). + ### Obsidian Open this directory in Obsidian for: diff --git a/VERSION b/VERSION index b1b25a5..197c4d5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.2.2 +2.4.0 diff --git a/instructions/mcp-read-server.md b/instructions/mcp-read-server.md new file mode 100644 index 0000000..d2d10b1 --- /dev/null +++ b/instructions/mcp-read-server.md @@ -0,0 +1,108 @@ +--- +type: types/instruction.md +name: mcp-read-server +description: Run and keep current the MCP read server that serves this wiki to a second consumer +--- + +# Running the MCP read server + +Chemenu has a second consumer. `search`, `types`, `describe_type`, `lint` and `status` are +served over MCP to callers that are not this terminal - the CLI and the server are two adapters +over one core (`chemenu.api.Corpus`), not a CLI with a network interface bolted on. + +This document is about *operating* it: how to start it, what has to be true of the checkout it +serves, and how that checkout stays current. What it exposes and why is in +[tools/CONTRACT.md](../tools/CONTRACT.md) and in the module's own docstring +(`tools/chemenu/mcp/server.py`). + +**Deployment is deliberately not here.** Which cluster, which ingress host, where the credential +lives - that is private infrastructure and this is a public repository. What is here is +everything an operator needs that is *true of the software* rather than of one installation. + +## When to run + +- Standing something up for a consumer that is not a terminal on this machine. +- Diagnosing an answer that looks stale, or one that disagrees with the CLI. +- Before pointing a new consumer at an existing server. + +## Steps + +1. **Install the server's dependency.** It is deliberately not in `requirements.txt`: an + instance that only uses the CLI should not be made to install pydantic, starlette, uvicorn + and cryptography to do it. + + ```bash + tools/.venv/bin/pip install -r tools/requirements-mcp.txt + ``` + +2. **Decide which checkout it serves.** The root resolves by precedence - an explicit `--root`, + then `$CHEMENU_ROOT`, then the checkout the package lives in. A deployment points at its + corpus with one variable and no code: + + ```bash + export CHEMENU_ROOT=/srv/chemenu + ``` + +3. **Take tracing out of the served tree.** The server refuses to start otherwise, and the + refusal is the point: telemetry defaults to on and writes under `reports/telemetry/` inside + the repo, which step 5's sync is entitled to wipe. Either is fine: + + ```bash + export WIKI_TRACE=0 # off + export WIKI_TRACE_DIR=/var/log/chemenu # or elsewhere, outside the corpus + ``` + +4. **Start it on the transport that matches what is in front of it.** + + ```bash + tools/.venv/bin/python -m chemenu.mcp # stdio + tools/.venv/bin/python -m chemenu.mcp --transport streamable-http # deployed + ``` + + `stdio` is for developing and testing without a network - one process per consumer, started + locally. `streamable-http` is what a deployed instance speaks, and the only one the + authentication middleware can sit in front of, because that middleware is an HTTP reverse + proxy. `sse` is reachable through the SDK and deliberately not offered: it is the superseded + remote transport, and building on it now only moves the migration later. + +5. **Keep the checkout current by polling, and keep it clean.** + + ```bash + git -C "$CHEMENU_ROOT" fetch --quiet origin && \ + git -C "$CHEMENU_ROOT" reset --hard --quiet origin/main + ``` + + Every few minutes, from a timer beside the server. Polling rather than a webhook on purpose: + it needs no inbound endpoint and no signature checking, which is a smaller surface than the + thing it would optimize. A webhook is a later optimization, not a starting point. + + `reset --hard` is load-bearing, not a convenience. The corpus cache reuses a parse while the + commit is unchanged and **refuses to cache a dirty tree at all**, so a checkout that has + drifted answers correctly but reparses on every request - and every answer it gives is + stamped `"commit": null`, because a dirty tree corresponds to no revision. + +## Decision points + +- **An answer looks stale?** Read `commit` in the response. If it names an old revision, the + sync is not running. If it is `null`, the served tree has uncommitted changes - something is + writing into the corpus that should not be. +- **The server disagrees with `wikitool` on the same query?** That is a defect, not a + configuration difference: the two go through the same functions and a golden test holds their + output together (`tools/chemenu/tests/test_mcp_server.py`). Check first that both are pointed + at the same root - `CHEMENU_ROOT` is easy to set for one and not the other. +- **Asked to expose a write tool?** There is none, and the way to add one is not a flag. The + server imports nothing under `chemenu.commands`, so `new`, `touch`, `xref`, `cite`, `publish` + and `migrate` are unreachable from it rather than filtered out of a list. Submitting documents + from outside is a different design with a quarantine in it - Gitea #32 - not a tool added + here. +- **Asked to rate-limit inside the server?** Rate limiting belongs in the middleware in front of + the process, next to authentication. Not the Iteration Budget Gate: that exists to stop an + agent *session* from iterating unnoticed over the wiki's state, which is why retrieval is + exempt from it, and using it as a rate limiter would dilute it into one. + +## Scope + +Not for setting up an instance ([setup-instance.md](setup-instance.md)) or a fresh clone +([bootstrap.md](bootstrap.md)). Not for the authentication or rate-limiting middleware, which is +infrastructure configuration rather than part of this repository. Not a write path: see the +decision point above. diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index 6175ca7..71feec7 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -50,7 +50,7 @@ tools/wikitool --help | `log append --op ingest\|query\|lint\|create\|update\|delete\|rename --title "..." [--body "..."\|--body-file path]` | Append a formatted entry to `kb/log.md` | | `log status` | Read-only: count `ingest` entries logged since the last `lint` entry - the deterministic trigger behind the Maintenance Schedule's "every 10 sources" full-lint cadence | | `lint [--json] [--markdown out.md] [--full] [--fail-on-error]` | Structural + provenance checks: broken wikilinks, dangling frontmatter references, orphan pages, index drift, schema gaps, duplicate titles, title mismatches, uncovered raw files, broken `raw_files:` refs, raw files claimed by more than one source page, unmarked provenance, citation/frontmatter drift, quote-limit overages (>2 blockquoted lines/page, advisory only). Prints only the sections that found something and always writes the full report to `reports/Lint Report .md` (or `--markdown`), naming the path - `--full` prints everything, `--json` prints the findings and writes nothing | -| `search [""] [--field ...] [--kind/--subtype/--collection/--tag ] [--regex] [--limit N] [--sort [-]] [--backend ] [--matches] [--json]` | Find pages in `kb/` without reading the index. Text search runs through a pluggable backend (`rg` today); `--field` predicates are evaluated on frontmatter - `f=v`, `f~substring`, `'f>=v'`, `'f:*'` (present), `'!f'` (absent), repeatable and ANDed. With no text this is a pure structured query. Results carry kind/summary/confidence so a hit can be judged without opening the page. Read-only, and **exempt from the Iteration Budget Gate** | +| `search [""] [--field ...] [--kind/--subtype/--collection/--tag ] [--regex] [--limit N] [--sort [-]] [--backend ] [--matches] [--json]` | Find pages in `kb/` without reading the index. Text search runs through a pluggable backend (`rg` today); `--field` predicates are evaluated on frontmatter - `f=v`, `f~substring`, `'f>=v'`, `'f:*'` (present), `'!f'` (absent), repeatable and ANDed. With no text this is a pure structured query. Results carry kind/summary/confidence so a hit can be judged without opening the page. A page whose frontmatter does not parse can match no positive predicate, so it is **named** rather than dropped: `--json` always carries an `unreadable` list of `{path, reason}` (usually empty), and the table form writes the same lines to stderr. `--regex` is applied by `rg` alone, whose engine is linear; the ranking boosts for title and summary are literal-containment only, so a non-literal pattern is ranked by match count. `rg` is killed after 30 s and reported as a failure. Read-only, and **exempt from the Iteration Budget Gate** | | `confidence decay [--apply]` | Recompute every page's derived `confidence` as `confidence_base * (1 - 0.01/month)`, floored at 0.2; dry-run by default | | `confidence init-base [--apply]` | One-time backfill: set `confidence_base` from the current `confidence` on pages that predate the derived-confidence model | | `sources coverage [--json]` | List raw files with no source page, broken `raw_files:` references, and legacy directory/URL-only source pages | @@ -84,8 +84,34 @@ tools/wikitool --help ## Design notes -- All commands operate on the real repo (paths resolved relative to this - file's location), so they can be run from any working directory. +- All commands operate on the real repo, so they can be run from any working + directory. The root is resolved by precedence - an explicit argument, then + `$CHEMENU_ROOT`, then a walk up from the package's own location - and the + walk-up is the default, so `tools/wikitool` with no variable set behaves + exactly as it always has. Nothing under the root is bound at import time: + `KB_DIR`, `RAW_DIR` and the rest follow whatever `ROOT` currently is, which + is what makes the half-repointed state (a moved `ROOT` with a stale `KB_DIR`) + unconstructible rather than merely discouraged. +- **The library boundary.** `chemenu.api.Corpus` is the in-process entry point: + it takes a corpus root, returns the same structures the `--json` forms print, + and raises `ChemenuError` where the CLI prints `ERROR` and exits 1. It is + read-only *structurally* - nothing under `chemenu.commands` is imported from + it, so `new`, `publish` and the rest are not reachable, rather than filtered. + The cores it calls (`search/service.py`, `lint_core.py`, `types_core.py`) + import no `typer` and no `rich`; the modules under `commands/` are the + terminal adapters over them. A second consumer is therefore a second adapter, + not a second implementation. +- **The MCP read server** (`chemenu/mcp/`) is that second adapter: `search`, + `types`, `describe_type`, `lint` and `status` over `chemenu.api`, on `stdio` + or `streamable-http`. Its dependency is optional and lives in + `requirements-mcp.txt`, so a CLI-only instance does not install it. There is + no write tool - nothing under `commands/` is importable from it, so the write + functions are unreachable rather than filtered - and every response carries + the commit it was computed from. Running it, and keeping its checkout + current, is [instructions/mcp-read-server.md](../instructions/mcp-read-server.md). + Authentication and rate limiting are middleware in front of the process, not + code here; the Iteration Budget Gate is deliberately not applied, because it + bounds an agent session rather than a user. - `new`/`xref`/`log append` only produce structurally-correct frontmatter and body skeletons/edits - the prose (Description, Summary, judgment calls about relationships) is still written by the LLM afterwards. @@ -151,7 +177,7 @@ is atomic, and whether a retry is safe. | `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 | | `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, a malformed `--field` predicate, an unknown field name, or an unknown `--backend` | Read-only | Fix the argument and retry. 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" | +| `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" | | `confidence decay --apply` / `init-base --apply` | Rare I/O error mid-loop | No - one write per page | Safe to retry freely; both recompute from `confidence_base` and never compound | | `sync` | The automatic rebase hit a real conflict (git failed) | No - fetch, then at most one merge/rebase attempt, aborted cleanly on failure | For a conflict: **do not retry, do not force** - resolve manually and re-run. **Exit 42, not 1**, when the rebase-review gate needs clearance: show the user the command's full output verbatim (upstream commits, the overlapping files, their diff) and stop; re-running with `--confirm-rebase ` clears it, and a wrong, invented, or superseded token exits 42 again with the current state. No remote configured, or one that cannot be reached, is not a failure - reported and skipped | | `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 | diff --git a/tools/README.md b/tools/README.md index 9bf16e4..59b74f7 100644 --- a/tools/README.md +++ b/tools/README.md @@ -34,20 +34,40 @@ tools/ wikitool entry point chemenu/ cli.py Typer app: registers every command, runs the budget gate - config.py repo layout constants (ROOT, RAW_DIR, KB_DIR, WORK_DIR, ...) + config.py repo layout: root resolution and every path under it + api.py the in-process entry point - point Chemenu at a corpus and read it + errors.py ChemenuError / ValidationError / BackendError + corpus_cache.py one parsed corpus per commit, never cached while the tree is dirty kb_scan.py page iteration/loading over kb/ kb_collections.py collection discovery (a directory with COLLECTION.md) type_resolver.py type-spec loading and schema resolution + lint_core.py the lint checks and the report, with no CLI attached + types_core.py type-spec listing/description, with no CLI attached sections.py the section headings the tool reads and writes in a page body markdown_code.py masks code spans/fences so a page may show wiki notation, not only use it version.py the stack version: VERSION, the release stamp, the compatibility rule kb_state.py the KB version (.wikitool-kb.json) and the migration chain corpus_diff.py invariant comparison of kb/ between two revisions - search/ pluggable search backends (base protocol, ripgrep, filters, fuse) - commands/ one module per command or command group + search/ pluggable search backends, plus service.py - the search core + commands/ one module per command or command group: the terminal adapters tests/ pytest suite ``` +**Two consumers, one core.** The CLI is not the only caller any more. The cores +(`search/service.py`, `lint_core.py`, `types_core.py`) hold what decides an +answer and import no `typer` and no `rich`; the modules under `commands/` turn +those values into terminal output and those exceptions into exit codes. +`api.Corpus` is the in-process entry point over the same functions - it takes a +corpus root, returns exactly the structures the `--json` forms print, and +raises instead of exiting. A second consumer is therefore a second adapter +rather than a second implementation, and the write commands are unreachable +from `api` because nothing under `commands/` is imported there. + +**Which corpus.** The root resolves by precedence: an explicit argument, then +`$CHEMENU_ROOT`, then a walk up from the package's own location. The walk-up is +the default, so the CLI is unaffected by any of this. Nothing under the root is +bound at import time - `KB_DIR` and friends follow whatever `ROOT` currently is. + ## Adding a command 1. Write the module under `chemenu/commands/`. A group is a `typer.Typer()` diff --git a/tools/chemenu/api.py b/tools/chemenu/api.py new file mode 100644 index 0000000..914cde5 --- /dev/null +++ b/tools/chemenu/api.py @@ -0,0 +1,202 @@ +"""The in-process entry point: point Chemenu at a corpus and read from it. + +This is the seam the MCP server (Gitea #19) is built on, and the reason it is +worth having as its own module rather than as "call the command functions +yourself": it fixes the two things that made an in-process caller a second-class +one. + +**A corpus you name, not the one this package happens to sit in.** `Corpus` +holds the root, threads it into the loader and the search backend, and points +`config` at it for the duration of each call so the parts that reach for +`config` directly - the `TypeResolver` singleton, which has to find `types/` - +follow too. A test proves no path of the developer's checkout is read while a +foreign root is set. + +**Values and exceptions, not exit codes.** The functions return the same +structures the CLI's `--json` forms print - one wire contract, with the CLI as +its executable specification - and raise `ChemenuError` where the CLI would +print `ERROR` and leave through `typer.Exit(1)`. + +Read-only, structurally: nothing under `chemenu.commands` is imported, so `new`, +`touch`, `xref`, `cite`, `publish`, `migrate` and `version bump` are not +reachable from here at all. That is the property #19 asks for - the write +functions do not exist in this surface rather than being filtered out of it. + +Import cost is the whole read core and nothing else: `yaml` and `jsonschema`, +plus the standard library. No `typer`, no `rich`. +""" +from __future__ import annotations + +import datetime +from pathlib import Path +from typing import Any, Iterable, Optional + +from chemenu import config +from chemenu.corpus_cache import CorpusCache +from chemenu.errors import BackendError, ChemenuError, ValidationError +from chemenu.lint_core import run_lint +from chemenu.search import filters +from chemenu.search.registry import resolve +from chemenu.search.service import run_search, unreadable_pages +from chemenu.search.types import Predicate, SearchQuery +from chemenu.types_core import describe_type, list_types + +# Distinguishes "the caller did not pass a revision" from "the caller passed +# None", which is itself a meaningful answer: no commit, because the tree is +# dirty or is not a checkout. +_UNREAD = object() + +__all__ = [ + "Corpus", + "ChemenuError", + "ValidationError", + "BackendError", +] + + +class Corpus: + """One corpus tree, read repeatedly. + + `root` follows `config.resolve_root()`: an explicit path, else + `$CHEMENU_ROOT`, else the checkout this package lives in. `kb_dir` defaults + to `/kb` and is separate only because the search backend already + distinguishes the two. + + Holds a `CorpusCache`, so a long-lived caller parses the corpus once per + commit instead of once per request - and never answers from a superseded + parse, because a dirty tree is not cached. Not thread-safe: a server serving + concurrent requests holds the lock, for the reason given in + `chemenu/corpus_cache.py`. + """ + + def __init__(self, root: Optional[Path | str] = None, kb_dir: Optional[Path | str] = None): + self.root = config.resolve_root(root) + self.kb_dir = Path(kb_dir) if kb_dir is not None else self.root / "kb" + self._cache = CorpusCache(self.kb_dir, self.root) + + @property + def revision(self) -> Optional[str]: + """The commit every answer from this corpus is stamped with, or None + when the tree is dirty or is not a git checkout - in which case the + answer corresponds to no commit, and says so.""" + return self._cache.current_revision() + + def _rooted(self): + """Point `config` at this corpus for the duration of one call. + + Threading a root through every argument gets the search backend and the + corpus loader, and misses the module-level `TypeResolver` singleton - + which resolves `types/` and is what `Page.kind` goes through. Without + this, a foreign corpus is read with *this* checkout's type specs, and + the page's `kind` is an answer about the wrong instance. + + Process-wide while open, so `Corpus` inherits `config.rooted()`'s + thread-safety constraint: one lock per process, held by the caller. + """ + return config.rooted(self.root) + + def _stamp( + self, payload: dict[str, Any], revision: Optional[str] = _UNREAD + ) -> dict[str, Any]: + """Every response carries the revision it was computed from. + + A stale checkout otherwise answers confidently and wrongly, which is the + failure `SOUL.md` names as the cardinal one. The stamp turns a silent + stale answer into a visible one. + + A caller that loaded the corpus passes the revision it got back, rather + than letting this ask again: between the load and the stamp the tree can + move, and the honest answer is the revision the pages actually came + from. Callers that read no pages (`types`) ask for the current one. + """ + if revision is _UNREAD: + revision = self._cache.current_revision() + payload["commit"] = revision + payload["as_of"] = datetime.datetime.now(datetime.timezone.utc).isoformat() + return payload + + def search( + self, + text: Optional[str] = None, + predicates: Iterable[str] = (), + regex: bool = False, + limit: int = 20, + sort: Optional[str] = None, + backend: Optional[str] = None, + ) -> dict[str, Any]: + """`wikitool search --json`, as a value. + + `predicates` takes the raw `--field` strings, so the CLI and this share + one parser and cannot drift on what `confidence<0.6` means. + """ + raw = list(predicates) + if not text and not raw: + raise ValidationError( + "Nothing to search for: give a query, or at least one predicate." + ) + parsed: tuple[Predicate, ...] = tuple(filters.parse_predicate(p) for p in raw) + backends = resolve(backend, self.kb_dir, self.root) + query = SearchQuery( + text=text, predicates=parsed, regex=regex, limit=limit, sort=sort + ) + + with self._rooted(): + pages, revision = self._cache.load() + hits = run_search(query, pages, backends, self.kb_dir) + return self._stamp({ + "query": text, + "predicates": [p.render() for p in parsed], + "backend": ",".join(b.name for b in backends), + "count": len(hits), + "results": [hit.as_dict() for hit in hits], + "unreadable": unreadable_pages(pages), + }, revision) + + def lint(self) -> dict[str, Any]: + """`wikitool lint --json`, as a value. + + The JSON form only - `lint` without a flag writes a report into + `reports/`, and a read surface does not write into the tree it is + reading. + """ + with self._rooted(): + return self._stamp(run_lint(self.kb_dir)) + + def types(self) -> dict[str, Any]: + """`wikitool types list --json`, as a value.""" + with self._rooted(): + return self._stamp({"types": list_types()}) + + def describe_type(self, name: str) -> dict[str, Any]: + """`wikitool types describe --json`, as a value. Raises + `UnknownType` (a `ValidationError`) for a name that does not exist.""" + with self._rooted(): + return self._stamp(describe_type(name)) + + def status(self) -> dict[str, Any]: + """A composed snapshot: how big the corpus is and what lint says about + it, without the full report. + + Composed here on purpose. There is no `wikitool status` to wrap - + `wiki-status` is a *skill* that assembles `kb/index.md`, `lint` and + `kb/log.md` - so this is a new surface, and saying so is what keeps + anyone from looking for the CLI command it mirrors. + """ + with self._rooted(): + pages, revision = self._cache.load() + report = run_lint(self.kb_dir) + collections: dict[str, int] = {} + for page in pages.values(): + name = filters.collection_of(page.path, self.kb_dir) + if name: + collections[name] = collections.get(name, 0) + 1 + return self._stamp({ + "pages": len(pages), + "collections": dict(sorted(collections.items())), + "findings": { + key: len(value) + for key, value in sorted(report.items()) + if isinstance(value, list) + }, + "unreadable": unreadable_pages(pages), + }, revision) diff --git a/tools/chemenu/commands/doctor.py b/tools/chemenu/commands/doctor.py index dca4f3c..a80077c 100644 --- a/tools/chemenu/commands/doctor.py +++ b/tools/chemenu/commands/doctor.py @@ -254,12 +254,19 @@ def check_publish_remotes() -> Check: configured and no allowlist. That is the shape a private instance has after it adds the public upstream, and it is exactly when a wrong `--remote` stops being a typo and starts being a disclosure. + + Both absent states say **armed** or **not armed** rather than only naming + the file. AGENTS.md lists this among the three limits enforced in code, so a + line that reports the file's absence and leaves the reader to infer what + that means about the gate is how a checkout ends up trusting a safeguard + that is not running - which is worse than having none. """ urls = git_publish.read_allowed_push_urls() if urls is not None: return Check( "publish-remotes", "OK", - f"{len(urls)} allowed push target(s) in {config.PUBLISH_REMOTES_FILENAME}", + f"Gate armed: {len(urls)} allowed push target(s) in " + f"{config.PUBLISH_REMOTES_FILENAME}", ) result = subprocess.run( ["git", "remote"], cwd=config.ROOT, capture_output=True, text=True @@ -268,13 +275,15 @@ def check_publish_remotes() -> Check: if len(remotes) > 1: return Check( "publish-remotes", "WARN", - f"{len(remotes)} remotes ({', '.join(remotes)}) and no publish allowlist", + f"Gate not armed: {len(remotes)} remotes ({', '.join(remotes)}) and no " + f"{config.PUBLISH_REMOTES_FILENAME} - every one of them is a legal publish target", f"Create {config.PUBLISH_REMOTES_FILENAME} naming the push URL this checkout " "may publish to - see instructions/gates.md", ) return Check( "publish-remotes", "OK", - f"No {config.PUBLISH_REMOTES_FILENAME} (unrestricted; one remote configured)", + f"Gate not armed: no {config.PUBLISH_REMOTES_FILENAME} - any push target passes " + "(1 remote configured, nothing to confuse it with)", ) diff --git a/tools/chemenu/commands/lint.py b/tools/chemenu/commands/lint.py index 2474a74..570a8b8 100644 --- a/tools/chemenu/commands/lint.py +++ b/tools/chemenu/commands/lint.py @@ -1,16 +1,12 @@ -"""Deterministic structural health checks for the wiki. +"""`wikitool lint` - the terminal adapter over `chemenu.lint_core`. -This intentionally covers only what can be computed mechanically: broken -wikilinks, orphan pages, index/page drift, frontmatter schema gaps, and -filename/title mismatches. Semantic judgment (contradictions, staleness, -what's worth writing about next) stays with the LLM - this report gives it a -verified factual foundation instead of requiring it to re-derive these facts -by reading every page. +The checks, the report and the hard-error rule live in `chemenu/lint_core.py`, +which imports no CLI machinery. This module owns only what a terminal needs: +the flags, where the report file lands, and the exit code. """ from __future__ import annotations import json -from datetime import date from pathlib import Path from typing import Optional @@ -18,416 +14,32 @@ import typer from chemenu import config from chemenu.commands._util import rel_path, success -from chemenu.frontmatter_io import frontmatter_error -from chemenu.markdown_code import strip_code_spans -from chemenu.provenance import broken_raw_refs as find_broken_raw_refs -from chemenu.provenance import duplicate_raw_file_owners as find_duplicate_raw_file_owners -from chemenu.provenance import extract_inline_cites -from chemenu.provenance import legacy_citation_markers as find_legacy_citation_markers -from chemenu.provenance import legacy_source_pages as find_legacy_source_pages -from chemenu.provenance import orphan_footnote_defs as find_orphan_footnote_defs -from chemenu.provenance import uncovered_raw_files as find_uncovered_raw_files -from chemenu.provenance import undefined_footnote_refs as find_undefined_footnote_refs -from chemenu.kb_scan import ( - GENERATED_INDEX, - WIKILINK_RE, - build_link_graph, - find_duplicate_title_paths, - inbound_links, - load_kb_pages, -) -from chemenu.type_resolver import resolver - -# Style guide's one mechanically-checkable rule (hard oracle: a plain count). -# The rest of the style guide (tone, AI-phrase avoidance) is a soft/proxy judgment -# and stays with the LLM - see wiki-manage/wiki-ingest skill guidance, not lint. -# -# The unit is a quote, not a `>` line. It used to be the line, which measured -# the wrap width the rule has no opinion about: one quotation written long -# counted 1 and the same quotation wrapped at 100 columns counted 4. An author -# who took the finding seriously made the page harder to read to quiet it. -QUOTE_LIMIT = 2 - -# How many hub pages `most_linked` reports. Purely informational (wiki-status -# surfaces it); not a finding, so the cutoff only bounds report size. -MOST_LINKED_COUNT = 10 - - -def count_quote_blocks(body: str) -> int: - """How many distinct blockquotes `body` carries. - - A run of consecutive `>` lines is one quote; a blank line or any - non-quoted line ends it. Code is masked out first, so a `>` inside a - fenced shell transcript is a prompt, not a quotation. - - Lazy continuation - a quote whose wrapped lines drop the `>` - reads here - as two quotes rather than one. That over-counts in the direction the limit - already errs on, and the corpus prefixes every line, so the alternative - (tracking paragraph state) buys nothing. - """ - count, in_quote = 0, False - for line in strip_code_spans(body).splitlines(): - is_quote = line.lstrip().startswith(">") - if is_quote and not in_quote: - count += 1 - in_quote = is_quote - return count - - -def run_lint(kb_dir: Path) -> dict: - pages = load_kb_pages(kb_dir) - duplicate_titles = find_duplicate_title_paths(kb_dir, config.ROOT) - - # Pages whose frontmatter can't be parsed read back as `{}` everywhere - # else, which would let them slip past every frontmatter-driven check - # below with no finding at all - so they are detected explicitly. - frontmatter_errors = [] - for title, page in sorted(pages.items()): - reason = frontmatter_error(page.path) - if reason is None and not page.frontmatter.get("type"): - reason = "missing `type:` field" - if reason is not None: - frontmatter_errors.append({"page": title, "error": reason}) - - graph = build_link_graph(pages) - broken_links = [ - {"page": title, "target": target} - for title, targets in graph.items() - for target in sorted(targets) - if target not in pages - ] - - inbound = inbound_links({t: v for t, v in graph.items() if t != "index"}) - orphan_pages = sorted( - title - for title, sources in inbound.items() - if not sources - and title not in ("index", "log") - # comparison pages are not linked to by design; index.md is sufficient coverage - and pages[title].kind != "comparison" - ) - - # Same link graph, opposite end: the most-linked-to pages are the wiki's - # hubs. Reported (not judged) so `wiki-status` can show them without - # re-deriving the graph. - inbound_counts = {title: len(sources) for title, sources in inbound.items()} - most_linked = [ - {"page": title, "inbound": count} - for title, count in sorted(inbound_counts.items(), key=lambda kv: (-kv[1], kv[0])) - if count > 0 - ][:MOST_LINKED_COUNT] - - # The catalog is sharded: `kb/index.md` is a map carrying counts and links, - # and the page rows live in a generated INDEX.md per collection/area. Both - # halves have to be read, or every page reads as missing from the index. - index_text = "".join( - path.read_text(encoding="utf-8") - for path in [kb_dir / "index.md", *sorted(kb_dir.rglob(GENERATED_INDEX))] - if path.exists() - ) - index_links = {m.group(1).strip() for m in WIKILINK_RE.finditer(index_text)} - missing_from_index = sorted(set(pages) - index_links - {"index", "log"}) - dangling_index_entries = sorted(index_links - set(pages)) - - title_mismatches = [] - for title, page in sorted(pages.items()): - if page.kind not in ("entity", "concept"): - continue - h1 = page.h1_title - if h1 is not None and h1 != title: - title_mismatches.append({"page": title, "h1": h1}) - - unmarked_provenance = [] - for title, page in sorted(pages.items()): - if page.kind not in ("entity", "concept"): - continue - sources_list = page.frontmatter.get("sources") or [] - if not sources_list and page.frontmatter.get("provenance") != "general": - unmarked_provenance.append(title) - - citation_frontmatter_drift = [] - for title, page in sorted(pages.items()): - sources_list = set(page.frontmatter.get("sources") or []) - cited = {cited_title for cited_title, _file in extract_inline_cites(page.body)} - cited.discard(title) # a source page citing itself for a specific file within it is not drift - for missing_source in sorted(cited - sources_list): - citation_frontmatter_drift.append({"page": title, "cited_but_not_in_sources": missing_source}) - - legacy_citation_markers = find_legacy_citation_markers(pages) - undefined_footnote_refs = find_undefined_footnote_refs(pages) - orphan_footnote_defs = find_orphan_footnote_defs(pages) - - # The frontmatter half of the link graph. `broken_links` above only walks - # `[[wikilinks]]` in page *bodies*, so a `related:`/`sources:`/`entities:` - # entry naming a page that does not exist - a rename that was not - # propagated, a deleted page, or a URL pasted where a title belongs - used - # to pass every check. Which fields hold page titles is declared by each - # type-spec's `page_ref_fields:`, not hardcoded here. - dangling_frontmatter_refs = [] - for title, page in sorted(pages.items()): - type_path = page.frontmatter.get("type") - if not type_path: - continue - try: - ref_fields = resolver.get_page_ref_fields(type_path, page.path) - except ValueError: - continue # unresolvable type is already reported as type_resolution_errors - for field in ref_fields: - for target in page.frontmatter.get(field) or []: - if target not in pages: - dangling_frontmatter_refs.append( - {"page": title, "field": field, "target": target} - ) - - quote_limit_violations = [] - for title, page in sorted(pages.items()): - quote_count = count_quote_blocks(page.body) - if quote_count > QUOTE_LIMIT: - quote_limit_violations.append({"page": title, "quote_count": quote_count}) - - # Type system validation. Lint reports are not validated here: they are - # written to `reports/` outside kb/ and are never pages, so nothing this - # loop scans can be one. - invalid_type_paths = [] - type_resolution_errors = [] - schema_validation_errors = [] - - for title, page in sorted(pages.items()): - type_path = page.frontmatter.get("type") - if not type_path: - continue - - # Check if type path is valid - if not type_path.endswith('.md'): - invalid_type_paths.append({"page": title, "type": type_path, "error": "Type path must end with .md"}) - continue - - # Try to resolve and validate the type - try: - resolver.load_type_spec(type_path, page.path) - - # Try schema validation - try: - resolver.validate_frontmatter(page.frontmatter, type_path, page.path) - except ValueError as schema_error: - schema_validation_errors.append({"page": title, "type": type_path, "error": str(schema_error)}) - - except ValueError as resolution_error: - type_resolution_errors.append({"page": title, "type": type_path, "error": str(resolution_error)}) - - return { - "generated": date.today().isoformat(), - "page_count": len(pages), - "frontmatter_errors": frontmatter_errors, - "broken_links": broken_links, - "orphan_pages": orphan_pages, - "most_linked": most_linked, - "inbound_counts": inbound_counts, - "missing_from_index": missing_from_index, - "dangling_index_entries": dangling_index_entries, - "title_mismatches": title_mismatches, - "duplicate_titles": duplicate_titles, - "uncovered_raw_files": find_uncovered_raw_files(config.RAW_DIR, pages), - "broken_raw_refs": find_broken_raw_refs(pages), - "duplicate_raw_file_owners": find_duplicate_raw_file_owners(pages), - "legacy_source_pages": find_legacy_source_pages(pages), - "unmarked_provenance": unmarked_provenance, - "citation_frontmatter_drift": citation_frontmatter_drift, - "legacy_citation_markers": legacy_citation_markers, - "undefined_footnote_refs": undefined_footnote_refs, - "orphan_footnote_defs": orphan_footnote_defs, - "dangling_frontmatter_refs": dangling_frontmatter_refs, - "quote_limit_violations": quote_limit_violations, - "invalid_type_paths": invalid_type_paths, - "type_resolution_errors": type_resolution_errors, - "schema_validation_errors": schema_validation_errors, - } - - -def _section(lines: list[str], title: str, items: list, formatter) -> None: - lines.append(f"## {title}") - lines.append("") - if not items: - lines.append("None found.") - else: - for item in items: - lines.append(f"- {formatter(item)}") - lines.append("") - - -def render_markdown(report: dict) -> str: - lines = [f"# Structural Lint Report ({report['generated']})", ""] - lines.append(f"Scanned {report['page_count']} pages under `wiki/`. This report covers only") - lines.append("mechanically-verifiable structural issues; see the Semantic Review section") - lines.append("below for judgment calls the LLM should complete.") - lines.append("") - - _section( - lines, "Unreadable Frontmatter", report["frontmatter_errors"], - lambda i: f"[[{i['page']}]] - {i['error']}", - ) - _section( - lines, "Broken Wikilinks", report["broken_links"], - lambda i: f"[[{i['page']}]] links to missing [[{i['target']}]]", - ) - _section(lines, "Orphan Pages (no inbound links)", report["orphan_pages"], lambda i: f"[[{i}]]") - _section( - lines, f"Most-Linked Pages (top {MOST_LINKED_COUNT} hubs)", report["most_linked"], - lambda i: f"[[{i['page']}]] - {i['inbound']} inbound link(s)", - ) - _section(lines, "Pages Missing from index.md", report["missing_from_index"], lambda i: f"[[{i}]]") - _section(lines, "Dangling index.md Entries", report["dangling_index_entries"], lambda i: f"[[{i}]]") - _section( - lines, "Duplicate Titles (naming collisions)", report["duplicate_titles"], - lambda i: f"`{i['stem']}` -> {', '.join(f'`{p}`' for p in i['paths'])}", - ) - _section( - lines, "Filename / H1 Title Mismatches", report["title_mismatches"], - lambda i: f"[[{i['page']}]] H1 is '{i['h1']}'", - ) - _section( - lines, "Uncovered Raw Files (no source page)", report["uncovered_raw_files"], - lambda i: f"`{i}`", - ) - _section( - lines, "Broken raw_files References", report["broken_raw_refs"], - lambda i: f"[[{i['page']}]] -> `{i['raw_path']}` (does not exist)", - ) - _section( - lines, "Raw Files With More Than One Owner", report["duplicate_raw_file_owners"], - lambda i: f"`{i['raw_file']}` is claimed by " + ", ".join(f"[[{t}]]" for t in i["owners"]), - ) - _section( - lines, "Legacy source: Field (not yet migrated to raw_files:)", report["legacy_source_pages"], - lambda i: f"[[{i['page']}]] source: `{i['source']}` ({i['reason']})", - ) - _section( - lines, "Pages Missing provenance: general Marker", report["unmarked_provenance"], - lambda i: f"[[{i}]] has no sources and is not marked `provenance: general`", - ) - _section( - lines, "Citation / Frontmatter Drift", report["citation_frontmatter_drift"], - lambda i: f"[[{i['page']}]] cites [[{i['cited_but_not_in_sources']}]] inline but it is missing from frontmatter `sources:`", - ) - _section( - lines, "Legacy Citation Markers (pre-migration `^[[...]]`)", report["legacy_citation_markers"], - lambda i: f"[[{i['page']}]] still has `{i['marker']}` - run `wikitool cite add` and replace it with the `[^cite-id]` it prints", - ) - _section( - lines, "Undefined Footnote References", report["undefined_footnote_refs"], - lambda i: f"[[{i['page']}]] references `[^{i['ref']}]`, which has no `[^{i['ref']}]: [[...]]` definition", - ) - _section( - lines, "Orphan Footnote Definitions", report["orphan_footnote_defs"], - lambda i: f"[[{i['page']}]] defines `[^{i['id']}]` (-> [[{i['source']}]]) but nothing references it - run `wikitool cite sync`", - ) - _section( - lines, "Dangling Frontmatter References", report["dangling_frontmatter_refs"], - lambda i: f"[[{i['page']}]] `{i['field']}:` names `{i['target']}`, which is not a page", - ) - _section( - lines, "Invalid Type Paths", report["invalid_type_paths"], - lambda i: f"[[{i['page']}]] has type: `{i['type']}` - {i['error']}", - ) - _section( - lines, "Type Resolution Errors", report["type_resolution_errors"], - lambda i: f"[[{i['page']}]] type: `{i['type']}` - {i['error']}", - ) - _section( - lines, "Schema Validation Errors", report["schema_validation_errors"], - lambda i: f"[[{i['page']}]] type: `{i['type']}` - {i['error']}", - ) - _section( - lines, f"Pages Exceeding Quote Limit (>{QUOTE_LIMIT}/page)", report["quote_limit_violations"], - lambda i: f"[[{i['page']}]] has {i['quote_count']} quotes - trim or confirm they're load-bearing", - ) - - lines.append("## Semantic Review (LLM to complete)") - lines.append("") - lines.append("- Contradictions across pages: TODO") - lines.append("- Stale claims (unconfirmed >6 months): TODO") - lines.append("- Suggested new pages / missing cross-references: TODO") - lines.append("") - return "\n".join(lines) - - -# Sections that always carry content but are not findings, so the summary -# handles them separately: a hub list is a statistic, and the semantic review -# is the checklist that follows the report rather than part of it. -INFORMATIONAL_SECTIONS = ("Most-Linked Pages",) -SEMANTIC_REVIEW_SECTION = "Semantic Review" - - -def _split_sections(markdown: str) -> tuple[str, list[tuple[str, str]]]: - """Cut a rendered report into its preamble and (title, body) sections.""" - preamble, *rest = markdown.split("\n## ") - sections = [] - for part in rest: - title, _, body = part.partition("\n") - sections.append((title.strip(), body.strip())) - return preamble.rstrip(), sections - - -def render_summary(report: dict) -> str: - """The same report with the empty sections removed. - - On a healthy corpus the full report is better than 90% "None found.", so - reading it in the terminal means paging past the answer. The file on disk - stays complete - this is what gets printed, and the written path underneath - it is how the rest is reached without running lint a second time. - """ - preamble, sections = _split_sections(render_markdown(report)) - findings, trailing = [], [] - for title, body in sections: - if title.startswith(SEMANTIC_REVIEW_SECTION): - trailing.append((title, body)) - elif body != "None found." and not title.startswith(INFORMATIONAL_SECTIONS): - findings.append((title, body)) - lines = [preamble, ""] - if not findings: - lines += ["No structural findings.", ""] - for title, body in findings + trailing: - lines += [f"## {title}", "", body, ""] - return "\n".join(lines) - - -def default_report_path(report: dict) -> Path: - """Where a report goes when the caller names no path. - - `reports/` is derived and gitignored ([reports/CONTRACT.md]), so writing - here by default costs the tree nothing. - """ - return config.ROOT / "reports" / f"Lint Report {report['generated']}.md" - - -# Findings that make a tree structurally wrong rather than merely untidy. -# `orphan_pages` is deliberately absent: many pages are validly reachable -# through the index or navigation only. `quote_limit_violations` is advisory -# too - it flags a habit, not a broken tree. -# -# One definition, used by `lint --fail-on-error` and by the eval scorecard: if -# the two disagreed, a run could pass its score while lint refused it. -HARD_ERROR_KEYS = ( - "frontmatter_errors", - "broken_links", - "dangling_index_entries", - "duplicate_titles", - "broken_raw_refs", - "duplicate_raw_file_owners", - "legacy_source_pages", - "citation_frontmatter_drift", - "legacy_citation_markers", - "undefined_footnote_refs", - "orphan_footnote_defs", - "dangling_frontmatter_refs", - "invalid_type_paths", - "type_resolution_errors", - "schema_validation_errors", +from chemenu.lint_core import ( + HARD_ERROR_KEYS, + MOST_LINKED_COUNT, + QUOTE_LIMIT, + count_quote_blocks, + default_report_path, + has_hard_errors, + render_markdown, + render_summary, + run_lint, ) - -def has_hard_errors(report: dict) -> bool: - return any(report.get(key) for key in HARD_ERROR_KEYS) +# Re-exported: `from chemenu.commands.lint import run_lint` still resolves, and +# so does every other name the tests and sibling commands already import. +__all__ = [ + "HARD_ERROR_KEYS", + "MOST_LINKED_COUNT", + "QUOTE_LIMIT", + "count_quote_blocks", + "default_report_path", + "has_hard_errors", + "render_markdown", + "render_summary", + "run_lint", + "lint_command", +] def lint_command( diff --git a/tools/chemenu/commands/search.py b/tools/chemenu/commands/search.py index 7d9ec5e..d2e4838 100644 --- a/tools/chemenu/commands/search.py +++ b/tools/chemenu/commands/search.py @@ -21,90 +21,38 @@ Scope is `kb/` only. `instructions/` is discovered through from __future__ import annotations import json -from pathlib import Path import typer -from chemenu import config from chemenu.commands._util import fail, today_iso -from chemenu.frontmatter_io import read_page -from chemenu.kb_scan import iter_kb_pages -from chemenu.page import Page from chemenu.search import filters -from chemenu.search.base import page_key from chemenu.search.filters import PredicateError -from chemenu.search.fuse import reciprocal_rank_fusion from chemenu.search.registry import UnknownBackend, resolve -from chemenu.search.ripgrep import RipgrepFailed, RipgrepMissing, build_hit +from chemenu.search.ripgrep import RipgrepFailed, RipgrepMissing +from chemenu.search.service import ( + load_pages_by_path, + run_search, + sort_hits, + unreadable_pages, +) from chemenu.search.types import Predicate, SearchHit, SearchQuery +# Re-exported so `from chemenu.commands.search import run_search` keeps +# resolving. The core lives in `chemenu/search/service.py`, which imports no +# CLI machinery; this module is the terminal adapter over it. +__all__ = [ + "load_pages_by_path", + "run_search", + "sort_hits", + "unreadable_pages", + "render_table", + "search_command", +] + TITLE_WIDTH = 34 SUMMARY_WIDTH = 84 -def load_pages_by_path(kb_dir: Path | None = None, root: Path | None = None) -> dict[str, Page]: - """Every page under `kb/`, keyed by repo-relative path. - - Path-keyed rather than title-keyed on purpose: `load_kb_pages()` drops one - of two pages sharing a stem, and search should still find both - a - duplicate title is a lint finding, not a reason to hide a page. - """ - kb_dir = kb_dir or config.KB_DIR - root = root or config.ROOT - pages: dict[str, Page] = {} - for path in iter_kb_pages(kb_dir): - frontmatter, body = read_page(path) - pages[page_key(path, root)] = Page(path=path, frontmatter=frontmatter, body=body) - return pages - - -def _sort_key(hit: SearchHit, field: str): - value = hit.as_dict().get(field) - if value is None: - # Missing values sort last in either direction rather than crashing on - # a None comparison. - return (1, "") - if isinstance(value, (int, float)): - return (0, value) - return (0, str(value).lower()) - - -def sort_hits(hits: list[SearchHit], sort: str | None) -> list[SearchHit]: - """Sort by a hit field. A leading `-` reverses, e.g. `--sort -confidence`.""" - if not sort: - return hits - descending = sort.startswith("-") - field = sort.lstrip("-") - ordered = sorted(hits, key=lambda h: _sort_key(h, field), reverse=descending) - return ordered - - -def run_search( - query: SearchQuery, - pages: dict[str, Page], - backends: list, - kb_dir: Path | None = None, -) -> list[SearchHit]: - """Answer a query. Pure: no I/O beyond whatever a backend does.""" - filters.validate_fields(query.predicates, pages) - - if query.text: - rankings = [backend.search(query, pages) for backend in backends] - hits = rankings[0] if len(rankings) == 1 else reciprocal_rank_fusion(rankings) - allowed = filters.apply_predicates(pages, query.predicates, kb_dir) - hits = [hit for hit in hits if hit.path in allowed] - else: - selected = filters.apply_predicates(pages, query.predicates, kb_dir) - hits = [ - build_hit(page, key, [], query, backend="frontmatter", kb_dir=kb_dir) - for key, page in selected.items() - ] - hits.sort(key=lambda h: h.title.lower()) - - hits = sort_hits(hits, query.sort) - return hits[: query.limit] if query.limit else hits - - def _truncate(text: str, width: int) -> str: text = " ".join(text.split()) return text if len(text) <= width else text[: width - 1] + "\u2026" @@ -206,6 +154,8 @@ def search_command( except RipgrepFailed as exc: fail(str(exc)) + unreadable = unreadable_pages(pages) + if json_out: payload = { "generated": today_iso(), @@ -214,8 +164,17 @@ def search_command( "backend": ",".join(b.name for b in backends), "count": len(hits), "results": [hit.as_dict() for hit in hits], + # Always present, usually empty. A caller that has to look for the + # key to learn whether it should worry will not look. + "unreadable": unreadable, } typer.echo(json.dumps(payload, indent=2)) return typer.echo(render_table(hits, show_matches)) + for entry in unreadable: + typer.echo( + f"WARN unreadable frontmatter: {entry['path']} ({entry['reason']}) - " + "this page cannot match any --field predicate", + err=True, + ) diff --git a/tools/chemenu/commands/types_cmd.py b/tools/chemenu/commands/types_cmd.py index 976db47..b04f42a 100644 --- a/tools/chemenu/commands/types_cmd.py +++ b/tools/chemenu/commands/types_cmd.py @@ -7,35 +7,29 @@ into context on every skill invocation. A type-spec's own frontmatter declared `.schema.yaml` are the single source of truth; this command only formats what `TypeResolver` already resolves - it does not duplicate or re-derive any type knowledge. + +The resolving half lives in `chemenu/types_core.py`, which imports no CLI +machinery. This module is the terminal adapter over it. """ from __future__ import annotations import json -from typing import Any, Dict import typer from chemenu.commands._util import fail -from chemenu.type_resolver import resolver +from chemenu.types_core import UnknownType, describe_type, list_types app = typer.Typer(help="Discover and describe Chemenu type-spec contracts.") @app.command("list") -def list_types(json_out: bool = typer.Option(False, "--json", help="Print raw findings as JSON")): +def list_types_command( + json_out: bool = typer.Option(False, "--json", help="Print raw findings as JSON") +): """List every type-spec under types/, with its name, schema, subtype field (if any), base directory, and description.""" - rows: list[Dict[str, Any]] = [] - for type_path, frontmatter in resolver.list_type_specs(): - rows.append({ - "name": frontmatter.get("name"), - "type_path": type_path, - "schema": frontmatter.get("schema"), - "subtype_field": frontmatter.get("subtype_field"), - "root": frontmatter.get("root") or "kb", - "base_dir": frontmatter.get("base_dir"), - "description": frontmatter.get("description"), - }) + rows = list_types() if json_out: typer.echo(json.dumps(rows, indent=2)) @@ -53,7 +47,7 @@ def list_types(json_out: bool = typer.Option(False, "--json", help="Print raw fi @app.command("describe") -def describe_type( +def describe_type_command( name: str = typer.Argument(..., help="Type name, e.g. 'entity' (see `types list`)"), json_out: bool = typer.Option(False, "--json", help="Print raw findings as JSON"), ): @@ -61,51 +55,26 @@ def describe_type( with enums where declared), its subtype field if any, and its authoring body - the same information an LLM would otherwise gather by reading the raw type-spec and `.schema.yaml` files directly.""" - type_path = resolver.find_type_by_name(name) - if type_path is None: - available = sorted(fm.get("name") for _, fm in resolver.list_type_specs()) - fail(f"No type-spec named '{name}'. Available: {', '.join(available)}") - return # unreachable; keeps type-checkers happy about `type_path` below - - type_spec = resolver.load_type_spec(type_path) - frontmatter = type_spec["frontmatter"] - body = type_spec["body"] - schema = resolver.get_schema(type_path) - - fields: list[Dict[str, Any]] = [] - if schema is not None: - required = set(schema.get("required", [])) - for field_name, field_schema in schema.get("properties", {}).items(): - fields.append({ - "field": field_name, - "required": field_name in required, - "type": field_schema.get("type"), - "enum": field_schema.get("enum"), - }) + try: + described = describe_type(name) + except UnknownType as exc: + fail(str(exc)) + return # unreachable; keeps type-checkers happy about `described` below if json_out: - typer.echo(json.dumps({ - "name": frontmatter.get("name"), - "type_path": type_path, - "description": frontmatter.get("description"), - "schema": frontmatter.get("schema"), - "subtype_field": frontmatter.get("subtype_field"), - "base_dir": frontmatter.get("base_dir"), - "title_prefix": frontmatter.get("title_prefix"), - "fields": fields, - "body": body.strip(), - }, indent=2)) + typer.echo(json.dumps(described, indent=2)) return - typer.echo(f"# {frontmatter.get('name')} ({type_path})") - typer.echo(frontmatter.get("description", "")) + fields = described["fields"] + typer.echo(f"# {described['name']} ({described['type_path']})") + typer.echo(described["description"] or "") typer.echo("") - if frontmatter.get("subtype_field"): - typer.echo(f"subtype_field: {frontmatter['subtype_field']}") - if frontmatter.get("base_dir"): - typer.echo(f"base_dir: {frontmatter.get('root') or 'kb'}/{frontmatter['base_dir']}") - if frontmatter.get("title_prefix"): - typer.echo(f"title_prefix: {frontmatter['title_prefix']!r}") + if described["subtype_field"]: + typer.echo(f"subtype_field: {described['subtype_field']}") + if described["base_dir"]: + typer.echo(f"base_dir: {described['root']}/{described['base_dir']}") + if described["title_prefix"]: + typer.echo(f"title_prefix: {described['title_prefix']!r}") typer.echo("") if not fields: @@ -119,4 +88,4 @@ def describe_type( typer.echo("") typer.echo("## Authoring guidance") - typer.echo(body.strip()) + typer.echo(described["body"]) diff --git a/tools/chemenu/config.py b/tools/chemenu/config.py index 851b0d9..fb48639 100644 --- a/tools/chemenu/config.py +++ b/tools/chemenu/config.py @@ -4,32 +4,164 @@ The repo is a pipeline: `raw/` (untrusted input) -> `types/` + `tools/` (schema compiler) -> `kb/` (compiled knowledge) -> `reports/` (derived output). Only `kb/` is divided into collections; the other three stages are single-purpose directories. -Repo root is resolved by walking up from this file's location (tools/chemenu/config.py --> tools/ -> repo root), which is stable regardless of the caller's current working -directory. +Repo root is resolved in three steps - an explicit argument to `resolve_root()`, then +`$CHEMENU_ROOT`, then a walk up from this file's location (tools/chemenu/config.py -> +tools/ -> repo root). The walk-up stays the default, so `tools/wikitool` behaves exactly +as it always has; the two steps in front of it are what lets an in-process caller point +this package at a corpus it does not itself live inside. + +**Nothing below is bound at import time.** `ROOT` and every path derived from it are +resolved on each attribute access, through the module `__getattr__` at the bottom. They +used to be module constants, which had a failure mode worse than the limitation itself: +`monkeypatch.setattr(config, "ROOT", other)` repointed `ROOT` and left `KB_DIR` and +`RAW_DIR` aimed at wherever this file happens to sit, so a caller that believed it was +working on a target tree was in fact answering out of the developer's checkout. Resolving +on access makes the derived paths follow whatever `ROOT` currently is - including a +monkeypatched one - so the half-repointed state cannot be constructed. """ import os import subprocess +from contextlib import contextmanager from pathlib import Path -ROOT = Path(__file__).resolve().parents[2] +# The last resort, and the default every existing caller gets: the checkout this +# file is part of. +_PACKAGE_ROOT = Path(__file__).resolve().parents[2] -RAW_DIR = ROOT / "raw" -KB_DIR = ROOT / "kb" -TYPES_DIR = ROOT / "types" -REPORTS_DIR = ROOT / "reports" -WORK_DIR = ROOT / "work" -INSTRUCTIONS_DIR = ROOT / "instructions" +# Points this package at a corpus other than its own checkout. Registered in +# `_WIKITOOL_ENV` (tools/chemenu/tests/conftest.py), so the suite runs with it +# cleared and a test that wants it sets it itself. +ENV_ROOT = "CHEMENU_ROOT" -# Generated copies of the skill directories under `instructions/`. Both are -# gitignored: they are build output, and a fresh clone publishes them with -# `wikitool instructions sync` (see instructions/bootstrap.md). -AGENTS_SKILLS_DIR = ROOT / ".agents" / "skills" -CLAUDE_SKILLS_DIR = ROOT / ".claude" / "skills" -INDEX_FILE = KB_DIR / "index.md" -LOG_FILE = KB_DIR / "log.md" -PROVENANCE_FILE = KB_DIR / "provenance.md" +def resolve_root(explicit: "Path | str | None" = None) -> Path: + """The repo root, by the documented precedence: argument, then + `$CHEMENU_ROOT`, then the checkout this package lives in. + + An explicit argument wins because a caller serving two corpora cannot use a + process-wide variable to tell them apart; the variable exists for the case + where the caller is a whole process (a server, a CI job) and there is + nothing to pass it through. + """ + if explicit is not None: + return Path(explicit).expanduser().resolve() + from_env = os.environ.get(ENV_ROOT, "").strip() + if from_env: + return Path(from_env).expanduser().resolve() + return _PACKAGE_ROOT + + +def _root() -> Path: + """`ROOT` as it stands right now, honouring an assignment onto this module. + + Reads the module dict directly rather than `resolve_root()` so that a test + (or any caller) setting `config.ROOT` is what the derived paths follow. + That assignment is why the derived paths are computed here at all. + """ + assigned = globals().get("ROOT") + return Path(assigned) if assigned is not None else resolve_root() + + +# Everything under the root, as a name -> relative-path table rather than as +# assignments. One place to read, and the only place that has to know a derived +# path exists at all. +_DERIVED = { + "RAW_DIR": ("raw",), + "KB_DIR": ("kb",), + "TYPES_DIR": ("types",), + "REPORTS_DIR": ("reports",), + "WORK_DIR": ("work",), + "INSTRUCTIONS_DIR": ("instructions",), + # Generated copies of the skill directories under `instructions/`. Both are + # gitignored: they are build output, and a fresh clone publishes them with + # `wikitool instructions sync` (see instructions/bootstrap.md). + "AGENTS_SKILLS_DIR": (".agents", "skills"), + "CLAUDE_SKILLS_DIR": (".claude", "skills"), +} + +# The generated files, derived from `KB_DIR` rather than from the root: a +# caller that repoints only the corpus directory must not be left with a log +# and a catalog belonging to a different tree. +_KB_DERIVED = { + "INDEX_FILE": "index.md", + "LOG_FILE": "log.md", + "PROVENANCE_FILE": "provenance.md", +} + + +# Every name this module resolves rather than stores. Assigning one is +# supported - that is what makes the paths repointable at all - but the +# assignment has to be taken back afterwards, or it outlives the caller that +# made it. See `reset()`. +MANAGED_PATHS = ("ROOT", *_DERIVED, *_KB_DERIVED) + + +@contextmanager +def rooted(root: "Path | str"): + """Resolve every managed path under `root` for the duration of the block. + + Some things below the read core reach for `config` directly rather than + taking a root - the module-level `TypeResolver` singleton, which has to + find `types/`, is the one that matters - so pointing this package at + another corpus means pointing `config` at it, not only the functions that + accept an argument. + + **Process-wide while it is open, and therefore not thread-safe.** A caller + serving several corpora at once holds a lock around it, the same discipline + `CorpusCache` documents. That is a real constraint and not a hidden one: + `$CHEMENU_ROOT` is process-wide for the same reason, and the server this + exists for (Gitea #19) serves one checkout that a `git reset --hard` keeps + clean. + + Restores exactly what was there, including "nothing was assigned" - it must + not leave `ROOT` bound behind it, or it recreates the stale-binding bug in + the shape `reset()` describes. + """ + previous = {name: globals()[name] for name in MANAGED_PATHS if name in globals()} + reset() + globals()["ROOT"] = Path(root) + try: + yield Path(root) + finally: + reset() + globals().update(previous) + + +def reset() -> None: + """Drop every assignment onto a managed path name, back to resolution. + + The test suite calls this between tests, and it is not optional there. + `monkeypatch.setattr(config, "KB_DIR", tmp)` records the old value by + *reading* it - which resolves it - and its undo then writes that resolved + path back as a real attribute. The name is bound from then on, so the next + caller to repoint only `ROOT` gets a `KB_DIR` still aimed at the previous + tree: exactly the half-repointed state this module was rewritten to make + unconstructible, rebuilt by the cleanup rather than by the test. + """ + for name in MANAGED_PATHS: + globals().pop(name, None) + + +def __getattr__(name: str): + """Resolve `ROOT` and the paths under it on access (PEP 562). + + Only reached for names *not* in the module dict, so an explicit assignment + - `monkeypatch.setattr(config, "ROOT", tmp)` - keeps working and now also + carries the derived paths with it, which is the bug this replaces. + """ + if name == "ROOT": + return resolve_root() + if name in _DERIVED: + return _root().joinpath(*_DERIVED[name]) + if name in _KB_DERIVED: + kb_dir = globals().get("KB_DIR") + base = Path(kb_dir) if kb_dir is not None else _root() / "kb" + return base / _KB_DERIVED[name] + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + +def __dir__() -> list[str]: + return sorted([*globals(), "ROOT", *_DERIVED, *_KB_DERIVED]) # Files/patterns to ignore when scanning raw/ for ingest coverage. # CONTRACT.md is the layer's source contract, not source material. @@ -102,7 +234,7 @@ def default_author() -> str | None: try: result = subprocess.run( ["git", "config", "user.name"], - cwd=ROOT, + cwd=_root(), capture_output=True, text=True, timeout=5, diff --git a/tools/chemenu/corpus_cache.py b/tools/chemenu/corpus_cache.py new file mode 100644 index 0000000..148fa7c --- /dev/null +++ b/tools/chemenu/corpus_cache.py @@ -0,0 +1,123 @@ +"""Load the corpus once per revision instead of once per query. + +`load_pages_by_path()` reads and parses every page under `kb/` on every call. +For a CLI that is the right shape - one call per process, nothing to reuse, and +a cache would only add a way to answer from a tree that has since changed. For +a long-lived reader (the MCP server) it is the opposite: the same corpus is +reparsed for every request, and the cost grows linearly with the corpus. + +So the cache is an object a caller holds, not a module-level dict that switches +itself on behind everyone's back. The CLI holds none and behaves exactly as +before; a resident process holds one. + +**The key is the commit SHA, and a dirty tree is never cached.** The SHA alone +would be a correctness bug in any checkout someone edits: a session that writes +a page and searches for it would be answered from the parse taken before the +write, with nothing about the SHA having changed. A clean tree is the state the +server actually runs in - it is kept that way by `git fetch && git reset +--hard` - so the fast path is the one that holds there, and every other tree +falls back to reloading. + +That same SHA is what a response is stamped with, which is deliberate: the +revision a caller is told about is by construction the revision its answer was +computed from. +""" +from __future__ import annotations + +import subprocess +from pathlib import Path +from typing import Optional + +from chemenu import config +from chemenu.page import Page + + +def head_commit(root: Optional[Path] = None) -> Optional[str]: + """The full SHA of `HEAD`, or None outside a git checkout.""" + result = _git(["rev-parse", "HEAD"], root) + if result is None or result.returncode != 0: + return None + return result.stdout.strip() or None + + +def is_dirty(root: Optional[Path] = None, path: Optional[Path] = None) -> bool: + """Whether the working tree has uncommitted changes under `path`. + + Errs toward dirty: if git cannot answer, the answer is "assume it changed". + A cache that treats "unknown" as clean serves stale pages, which is the one + outcome this module exists to prevent. + """ + root = root or config.ROOT + target = path or config.KB_DIR + try: + relative = Path(target).resolve().relative_to(Path(root).resolve()).as_posix() + except ValueError: + return True + result = _git(["status", "--porcelain", "--", relative], root) + if result is None or result.returncode != 0: + return True + return bool(result.stdout.strip()) + + +def _git(args: list[str], root: Optional[Path] = None): + try: + return subprocess.run( + ["git", *args], + cwd=root or config.ROOT, + capture_output=True, + text=True, + timeout=10, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + + +class CorpusCache: + """One parsed corpus, reused while the checkout stays on the same commit. + + Not thread-safe by itself: a caller serving concurrent requests holds the + lock. Kept out of here because the locking discipline belongs to whoever + owns the request loop, and a lock hidden in a cache is one nobody can see + when they need to reason about it. + """ + + def __init__(self, kb_dir: Optional[Path] = None, root: Optional[Path] = None): + self.kb_dir = kb_dir + self.root = root + self._pages: Optional[dict[str, Page]] = None + self._revision: Optional[str] = None + + @property + def revision(self) -> Optional[str]: + """The commit the cached corpus was read at, or None if nothing is + cached (including because the tree was dirty).""" + return self._revision + + def current_revision(self) -> Optional[str]: + """The commit this corpus would be cached under right now: `HEAD` on a + clean tree, None on a dirty one or outside git. None means uncacheable, + which is why it is also what a caller should report as "no revision" - + an answer read out of a dirty tree does not correspond to any commit.""" + root = self.root or config.ROOT + if is_dirty(root, self.kb_dir or config.KB_DIR): + return None + return head_commit(root) + + def load(self) -> tuple[dict[str, Page], Optional[str]]: + """(pages, revision). Reparses whenever the revision is not the cached + one, and on every call while the tree is dirty.""" + # Imported here rather than at module level: `commands.search` pulls in + # typer, and this module is meant to be importable by a library caller + # that has no CLI. Removing that edge properly is the library-boundary + # work, not this file's job. + from chemenu.commands.search import load_pages_by_path + + revision = self.current_revision() + if revision is None or revision != self._revision or self._pages is None: + pages = load_pages_by_path(self.kb_dir, self.root) + if revision is None: + self._pages, self._revision = None, None + return pages, None + self._pages, self._revision = pages, revision + return self._pages, self._revision diff --git a/tools/chemenu/errors.py b/tools/chemenu/errors.py new file mode 100644 index 0000000..6372403 --- /dev/null +++ b/tools/chemenu/errors.py @@ -0,0 +1,31 @@ +"""The exception contract at the library boundary. + +The CLI reports a bad argument by printing an `ERROR` line and leaving through +`typer.Exit(1)`. That is the right answer for a terminal and the wrong one for +an in-process caller, which gets an exit code where it expected a value, plus +module-global state (`_util._declined`) surviving into its next call. + +So the core raises, and the CLI adapter translates. `ChemenuError` is the one +class a library caller has to know; the two below it separate "your input was +wrong, a different argument would work" from "the machinery underneath failed", +which is the same distinction the CLI's exit codes draw. + +`ValidationError` also inherits `ValueError`. Not for elegance: `PredicateError` +was a `ValueError` before this existed, and callers catch it that way. +""" +from __future__ import annotations + + +class ChemenuError(Exception): + """Base for every error this package raises deliberately.""" + + +class ValidationError(ChemenuError, ValueError): + """The caller's input was rejected. Re-running unchanged fails identically; + the CLI renders this as its exit-1 `ERROR` line.""" + + +class BackendError(ChemenuError, RuntimeError): + """A dependency the core relies on was missing or failed - `rg` absent, a + search that had to be killed. Not the caller's argument, and not + necessarily permanent.""" diff --git a/tools/chemenu/frontmatter_io.py b/tools/chemenu/frontmatter_io.py index 2cf051f..47118e6 100644 --- a/tools/chemenu/frontmatter_io.py +++ b/tools/chemenu/frontmatter_io.py @@ -16,29 +16,137 @@ from typing import Any import yaml +from chemenu.errors import ValidationError + +try: # pragma: no cover - which branch runs depends on the host's libyaml + from yaml import CSafeLoader as _Loader +except ImportError: # pragma: no cover + from yaml import SafeLoader as _Loader + FRONTMATTER_RE = re.compile(r"\A---\n(.*?)\n---\n?(.*)\Z", re.DOTALL) +# Frontmatter is a flat block of scalars and one-line lists. Real pages sit +# well under a kilobyte, so this is not a budget anyone writes against - it is +# the bound that keeps parse cost proportional to the corpus rather than to +# whatever a single file claims to be. It matters because the parser is on the +# read path, and the read path is what gets exposed. +MAX_FRONTMATTER_BYTES = 64 * 1024 + +# YAML anchors and aliases are refused outright rather than budgeted. A page's +# frontmatter has no use for them, and alias expansion is where a small +# document becomes an enormous object graph: 267 bytes of nested aliases +# compose in 0.2 ms into 672,603 nodes, growing 9**n with nesting depth at +# constant parse time. A size limit alone does not touch that, because the +# input stays small - see `ALIAS_BOMB` in the tests. +# +# The check runs on the *event* stream (`yaml.parse`), which is streaming and +# resolves nothing - so asking the question costs O(text) and never triggers +# the expansion it is asking about. `"*"` is a necessary character in any alias +# node, so its absence proves absence without parsing at all, which is the case +# every real page takes. +_ALIAS_HINT = "*" + + +class FrontmatterError(ValidationError): + """Frontmatter that cannot be used: missing, malformed, oversized, or + refused by a limit. Raised only by the strict entry points - the permissive + ones report it as a string instead.""" + + +def _load_frontmatter(fm_text: str) -> tuple[dict[str, Any] | None, str | None]: + """Parse one frontmatter block into (mapping, error). Exactly one is None. + + The single parser behind both `read_page()` and `frontmatter_error()`. They + used to have one each, which is how the permissive path could degrade to + `{}` for a reason the strict path described differently - and how every + caller wanting both answers read and parsed the file twice. + """ + encoded = len(fm_text.encode("utf-8")) + if encoded > MAX_FRONTMATTER_BYTES: + return None, ( + f"frontmatter is {encoded} bytes, over the {MAX_FRONTMATTER_BYTES}-byte limit" + ) + if _ALIAS_HINT in fm_text: + try: + for event in yaml.parse(fm_text, Loader=_Loader): + if isinstance(event, yaml.AliasEvent): + return None, ( + "frontmatter uses a YAML alias (`*" + f"{event.anchor}`); anchors and aliases are not allowed here" + ) + except yaml.YAMLError as exc: + return None, f"invalid YAML frontmatter: {_first_line(exc)}" + except RecursionError: + return None, "frontmatter is nested too deeply to parse" + try: + parsed = yaml.load(fm_text, Loader=_Loader) + except yaml.YAMLError as exc: + return None, f"invalid YAML frontmatter: {_first_line(exc)}" + except RecursionError: + # PyYAML composes recursively, so deep nesting exhausts the interpreter + # stack rather than raising a YAMLError. Unbounded nesting is bounded by + # MAX_FRONTMATTER_BYTES; this catches what fits under it. + return None, "frontmatter is nested too deeply to parse" + if parsed is None: + return None, "empty frontmatter block" + if not isinstance(parsed, dict): + return None, f"frontmatter is {type(parsed).__name__}, expected a mapping" + return parsed, None + + +def _first_line(exc: Exception) -> str: + text = str(exc) + return text.splitlines()[0] if text else exc.__class__.__name__ + + +def read_page_with_error(path: Path) -> tuple[dict[str, Any], str, str | None]: + """(frontmatter, body, error) - the permissive read, with the reason it was + permissive handed back instead of dropped. + + A page whose YAML is broken reads as `{}`, and a `{}` page then has no + `confidence` and no `kind`: it drops out of `--field confidence<0.6` - + precisely the query meant to find pages in bad shape - while looking to the + caller like a page that simply did not match. Returning the reason is what + lets a caller say so instead of losing the page quietly. + """ + text = path.read_text(encoding="utf-8") + match = FRONTMATTER_RE.match(text) + if not match: + return {}, text, "no `---` frontmatter block" + fm_text, body = match.group(1), match.group(2) + parsed, error = _load_frontmatter(fm_text) + if error == "empty frontmatter block": + # An empty block is a legitimate shape for the permissive read - it + # carries no fields, and there is nothing to lose. Only `lint` treats + # it as a finding. + return {}, body, error + return (parsed or {}), body, error + def read_page(path: Path) -> tuple[dict[str, Any], str]: """Return (frontmatter_dict, body) for a markdown file. If the file has no frontmatter block, returns ({}, full_text). Deliberately permissive: malformed YAML degrades to an empty dict so bulk - operations never crash on one bad page. Use `frontmatter_error()` (which - `wikitool lint` does) to surface those pages instead of losing them - silently. + operations never crash on one bad page. Use `read_page_with_error()` (search + does) or `frontmatter_error()` (`wikitool lint` does) to surface those pages + instead of losing them silently. """ - text = path.read_text(encoding="utf-8") - match = FRONTMATTER_RE.match(text) - if not match: - return {}, text - fm_text, body = match.group(1), match.group(2) - try: - frontmatter = yaml.safe_load(fm_text) or {} - except yaml.YAMLError: - frontmatter = {} - if not isinstance(frontmatter, dict): - frontmatter = {} + frontmatter, body, _ = read_page_with_error(path) + return frontmatter, body + + +def read_page_strict(path: Path) -> tuple[dict[str, Any], str]: + """`read_page()` that raises `FrontmatterError` instead of degrading. + + For any path that ingests frontmatter this instance did not write itself. + The permissive read is right for bulk operations over a corpus the operator + committed; it is wrong the moment the document arrives from outside, where + "unparseable" must stop the document rather than empty it. + """ + frontmatter, body, error = read_page_with_error(path) + if error is not None: + raise FrontmatterError(f"{path}: {error}") return frontmatter, body @@ -50,20 +158,7 @@ def frontmatter_error(path: Path) -> str | None: YAML is malformed (or whose frontmatter block is missing entirely) reads back as `{}` and then quietly slips past every frontmatter-driven check. """ - text = path.read_text(encoding="utf-8") - match = FRONTMATTER_RE.match(text) - if not match: - return "no `---` frontmatter block" - try: - parsed = yaml.safe_load(match.group(1)) - except yaml.YAMLError as exc: - reason = str(exc).splitlines()[0] if str(exc) else exc.__class__.__name__ - return f"invalid YAML frontmatter: {reason}" - if parsed is None: - return "empty frontmatter block" - if not isinstance(parsed, dict): - return f"frontmatter is {type(parsed).__name__}, expected a mapping" - return None + return read_page_with_error(path)[2] def _format_scalar(value: Any, flow: bool = False) -> str: @@ -132,7 +227,7 @@ def _round_trips_as_string(text: str, flow: bool = False) -> bool: """ probe, expected = (f"[{text}]", [text]) if flow else (text, text) try: - return yaml.safe_load(probe) == expected + return yaml.load(probe, Loader=_Loader) == expected except yaml.YAMLError: # Unparseable bare - quoting is exactly the fix. return False diff --git a/tools/chemenu/lint_core.py b/tools/chemenu/lint_core.py new file mode 100644 index 0000000..033f39a --- /dev/null +++ b/tools/chemenu/lint_core.py @@ -0,0 +1,431 @@ +"""The lint core, with no CLI attached. + +Split out of `commands/lint.py` for the reason given in +`chemenu/search/service.py`: `run_lint()` is a pure function over a corpus +directory, and it was sitting in a module that imports `typer` and `rich`, so +no in-process caller could reach it without the CLI head. + +Everything that decides *findings* lives here. Everything that decides *how a +terminal sees them* - the report file, the exit code, the flags - stays in +`commands/lint.py`. `render_markdown()` and `render_summary()` are on this side +of the line because the markdown report is a data product (it is what +`reports/` holds and what `kb/log.md` refers to), not terminal formatting. +""" +from __future__ import annotations + +from datetime import date +from pathlib import Path + +from chemenu import config +from chemenu.frontmatter_io import frontmatter_error +from chemenu.markdown_code import strip_code_spans +from chemenu.provenance import broken_raw_refs as find_broken_raw_refs +from chemenu.provenance import duplicate_raw_file_owners as find_duplicate_raw_file_owners +from chemenu.provenance import extract_inline_cites +from chemenu.provenance import legacy_citation_markers as find_legacy_citation_markers +from chemenu.provenance import legacy_source_pages as find_legacy_source_pages +from chemenu.provenance import orphan_footnote_defs as find_orphan_footnote_defs +from chemenu.provenance import uncovered_raw_files as find_uncovered_raw_files +from chemenu.provenance import undefined_footnote_refs as find_undefined_footnote_refs +from chemenu.kb_scan import ( + GENERATED_INDEX, + WIKILINK_RE, + build_link_graph, + find_duplicate_title_paths, + inbound_links, + load_kb_pages, +) +from chemenu.type_resolver import resolver + +# Style guide's one mechanically-checkable rule (hard oracle: a plain count). +# The rest of the style guide (tone, AI-phrase avoidance) is a soft/proxy judgment +# and stays with the LLM - see wiki-manage/wiki-ingest skill guidance, not lint. +# +# The unit is a quote, not a `>` line. It used to be the line, which measured +# the wrap width the rule has no opinion about: one quotation written long +# counted 1 and the same quotation wrapped at 100 columns counted 4. An author +# who took the finding seriously made the page harder to read to quiet it. + + +QUOTE_LIMIT = 2 + +# How many hub pages `most_linked` reports. Purely informational (wiki-status +# surfaces it); not a finding, so the cutoff only bounds report size. +MOST_LINKED_COUNT = 10 + + +def count_quote_blocks(body: str) -> int: + """How many distinct blockquotes `body` carries. + + A run of consecutive `>` lines is one quote; a blank line or any + non-quoted line ends it. Code is masked out first, so a `>` inside a + fenced shell transcript is a prompt, not a quotation. + + Lazy continuation - a quote whose wrapped lines drop the `>` - reads here + as two quotes rather than one. That over-counts in the direction the limit + already errs on, and the corpus prefixes every line, so the alternative + (tracking paragraph state) buys nothing. + """ + count, in_quote = 0, False + for line in strip_code_spans(body).splitlines(): + is_quote = line.lstrip().startswith(">") + if is_quote and not in_quote: + count += 1 + in_quote = is_quote + return count + + +def run_lint(kb_dir: Path) -> dict: + pages = load_kb_pages(kb_dir) + duplicate_titles = find_duplicate_title_paths(kb_dir, config.ROOT) + + # Pages whose frontmatter can't be parsed read back as `{}` everywhere + # else, which would let them slip past every frontmatter-driven check + # below with no finding at all - so they are detected explicitly. + frontmatter_errors = [] + for title, page in sorted(pages.items()): + reason = frontmatter_error(page.path) + if reason is None and not page.frontmatter.get("type"): + reason = "missing `type:` field" + if reason is not None: + frontmatter_errors.append({"page": title, "error": reason}) + + graph = build_link_graph(pages) + broken_links = [ + {"page": title, "target": target} + for title, targets in graph.items() + for target in sorted(targets) + if target not in pages + ] + + inbound = inbound_links({t: v for t, v in graph.items() if t != "index"}) + orphan_pages = sorted( + title + for title, sources in inbound.items() + if not sources + and title not in ("index", "log") + # comparison pages are not linked to by design; index.md is sufficient coverage + and pages[title].kind != "comparison" + ) + + # Same link graph, opposite end: the most-linked-to pages are the wiki's + # hubs. Reported (not judged) so `wiki-status` can show them without + # re-deriving the graph. + inbound_counts = {title: len(sources) for title, sources in inbound.items()} + most_linked = [ + {"page": title, "inbound": count} + for title, count in sorted(inbound_counts.items(), key=lambda kv: (-kv[1], kv[0])) + if count > 0 + ][:MOST_LINKED_COUNT] + + # The catalog is sharded: `kb/index.md` is a map carrying counts and links, + # and the page rows live in a generated INDEX.md per collection/area. Both + # halves have to be read, or every page reads as missing from the index. + index_text = "".join( + path.read_text(encoding="utf-8") + for path in [kb_dir / "index.md", *sorted(kb_dir.rglob(GENERATED_INDEX))] + if path.exists() + ) + index_links = {m.group(1).strip() for m in WIKILINK_RE.finditer(index_text)} + missing_from_index = sorted(set(pages) - index_links - {"index", "log"}) + dangling_index_entries = sorted(index_links - set(pages)) + + title_mismatches = [] + for title, page in sorted(pages.items()): + if page.kind not in ("entity", "concept"): + continue + h1 = page.h1_title + if h1 is not None and h1 != title: + title_mismatches.append({"page": title, "h1": h1}) + + unmarked_provenance = [] + for title, page in sorted(pages.items()): + if page.kind not in ("entity", "concept"): + continue + sources_list = page.frontmatter.get("sources") or [] + if not sources_list and page.frontmatter.get("provenance") != "general": + unmarked_provenance.append(title) + + citation_frontmatter_drift = [] + for title, page in sorted(pages.items()): + sources_list = set(page.frontmatter.get("sources") or []) + cited = {cited_title for cited_title, _file in extract_inline_cites(page.body)} + cited.discard(title) # a source page citing itself for a specific file within it is not drift + for missing_source in sorted(cited - sources_list): + citation_frontmatter_drift.append({"page": title, "cited_but_not_in_sources": missing_source}) + + legacy_citation_markers = find_legacy_citation_markers(pages) + undefined_footnote_refs = find_undefined_footnote_refs(pages) + orphan_footnote_defs = find_orphan_footnote_defs(pages) + + # The frontmatter half of the link graph. `broken_links` above only walks + # `[[wikilinks]]` in page *bodies*, so a `related:`/`sources:`/`entities:` + # entry naming a page that does not exist - a rename that was not + # propagated, a deleted page, or a URL pasted where a title belongs - used + # to pass every check. Which fields hold page titles is declared by each + # type-spec's `page_ref_fields:`, not hardcoded here. + dangling_frontmatter_refs = [] + for title, page in sorted(pages.items()): + type_path = page.frontmatter.get("type") + if not type_path: + continue + try: + ref_fields = resolver.get_page_ref_fields(type_path, page.path) + except ValueError: + continue # unresolvable type is already reported as type_resolution_errors + for field in ref_fields: + for target in page.frontmatter.get(field) or []: + if target not in pages: + dangling_frontmatter_refs.append( + {"page": title, "field": field, "target": target} + ) + + quote_limit_violations = [] + for title, page in sorted(pages.items()): + quote_count = count_quote_blocks(page.body) + if quote_count > QUOTE_LIMIT: + quote_limit_violations.append({"page": title, "quote_count": quote_count}) + + # Type system validation. Lint reports are not validated here: they are + # written to `reports/` outside kb/ and are never pages, so nothing this + # loop scans can be one. + invalid_type_paths = [] + type_resolution_errors = [] + schema_validation_errors = [] + + for title, page in sorted(pages.items()): + type_path = page.frontmatter.get("type") + if not type_path: + continue + + # Check if type path is valid + if not type_path.endswith('.md'): + invalid_type_paths.append({"page": title, "type": type_path, "error": "Type path must end with .md"}) + continue + + # Try to resolve and validate the type + try: + resolver.load_type_spec(type_path, page.path) + + # Try schema validation + try: + resolver.validate_frontmatter(page.frontmatter, type_path, page.path) + except ValueError as schema_error: + schema_validation_errors.append({"page": title, "type": type_path, "error": str(schema_error)}) + + except ValueError as resolution_error: + type_resolution_errors.append({"page": title, "type": type_path, "error": str(resolution_error)}) + + return { + "generated": date.today().isoformat(), + "page_count": len(pages), + "frontmatter_errors": frontmatter_errors, + "broken_links": broken_links, + "orphan_pages": orphan_pages, + "most_linked": most_linked, + "inbound_counts": inbound_counts, + "missing_from_index": missing_from_index, + "dangling_index_entries": dangling_index_entries, + "title_mismatches": title_mismatches, + "duplicate_titles": duplicate_titles, + "uncovered_raw_files": find_uncovered_raw_files(config.RAW_DIR, pages), + "broken_raw_refs": find_broken_raw_refs(pages), + "duplicate_raw_file_owners": find_duplicate_raw_file_owners(pages), + "legacy_source_pages": find_legacy_source_pages(pages), + "unmarked_provenance": unmarked_provenance, + "citation_frontmatter_drift": citation_frontmatter_drift, + "legacy_citation_markers": legacy_citation_markers, + "undefined_footnote_refs": undefined_footnote_refs, + "orphan_footnote_defs": orphan_footnote_defs, + "dangling_frontmatter_refs": dangling_frontmatter_refs, + "quote_limit_violations": quote_limit_violations, + "invalid_type_paths": invalid_type_paths, + "type_resolution_errors": type_resolution_errors, + "schema_validation_errors": schema_validation_errors, + } + + +def _section(lines: list[str], title: str, items: list, formatter) -> None: + lines.append(f"## {title}") + lines.append("") + if not items: + lines.append("None found.") + else: + for item in items: + lines.append(f"- {formatter(item)}") + lines.append("") + + +def render_markdown(report: dict) -> str: + lines = [f"# Structural Lint Report ({report['generated']})", ""] + lines.append(f"Scanned {report['page_count']} pages under `wiki/`. This report covers only") + lines.append("mechanically-verifiable structural issues; see the Semantic Review section") + lines.append("below for judgment calls the LLM should complete.") + lines.append("") + + _section( + lines, "Unreadable Frontmatter", report["frontmatter_errors"], + lambda i: f"[[{i['page']}]] - {i['error']}", + ) + _section( + lines, "Broken Wikilinks", report["broken_links"], + lambda i: f"[[{i['page']}]] links to missing [[{i['target']}]]", + ) + _section(lines, "Orphan Pages (no inbound links)", report["orphan_pages"], lambda i: f"[[{i}]]") + _section( + lines, f"Most-Linked Pages (top {MOST_LINKED_COUNT} hubs)", report["most_linked"], + lambda i: f"[[{i['page']}]] - {i['inbound']} inbound link(s)", + ) + _section(lines, "Pages Missing from index.md", report["missing_from_index"], lambda i: f"[[{i}]]") + _section(lines, "Dangling index.md Entries", report["dangling_index_entries"], lambda i: f"[[{i}]]") + _section( + lines, "Duplicate Titles (naming collisions)", report["duplicate_titles"], + lambda i: f"`{i['stem']}` -> {', '.join(f'`{p}`' for p in i['paths'])}", + ) + _section( + lines, "Filename / H1 Title Mismatches", report["title_mismatches"], + lambda i: f"[[{i['page']}]] H1 is '{i['h1']}'", + ) + _section( + lines, "Uncovered Raw Files (no source page)", report["uncovered_raw_files"], + lambda i: f"`{i}`", + ) + _section( + lines, "Broken raw_files References", report["broken_raw_refs"], + lambda i: f"[[{i['page']}]] -> `{i['raw_path']}` (does not exist)", + ) + _section( + lines, "Raw Files With More Than One Owner", report["duplicate_raw_file_owners"], + lambda i: f"`{i['raw_file']}` is claimed by " + ", ".join(f"[[{t}]]" for t in i["owners"]), + ) + _section( + lines, "Legacy source: Field (not yet migrated to raw_files:)", report["legacy_source_pages"], + lambda i: f"[[{i['page']}]] source: `{i['source']}` ({i['reason']})", + ) + _section( + lines, "Pages Missing provenance: general Marker", report["unmarked_provenance"], + lambda i: f"[[{i}]] has no sources and is not marked `provenance: general`", + ) + _section( + lines, "Citation / Frontmatter Drift", report["citation_frontmatter_drift"], + lambda i: f"[[{i['page']}]] cites [[{i['cited_but_not_in_sources']}]] inline but it is missing from frontmatter `sources:`", + ) + _section( + lines, "Legacy Citation Markers (pre-migration `^[[...]]`)", report["legacy_citation_markers"], + lambda i: f"[[{i['page']}]] still has `{i['marker']}` - run `wikitool cite add` and replace it with the `[^cite-id]` it prints", + ) + _section( + lines, "Undefined Footnote References", report["undefined_footnote_refs"], + lambda i: f"[[{i['page']}]] references `[^{i['ref']}]`, which has no `[^{i['ref']}]: [[...]]` definition", + ) + _section( + lines, "Orphan Footnote Definitions", report["orphan_footnote_defs"], + lambda i: f"[[{i['page']}]] defines `[^{i['id']}]` (-> [[{i['source']}]]) but nothing references it - run `wikitool cite sync`", + ) + _section( + lines, "Dangling Frontmatter References", report["dangling_frontmatter_refs"], + lambda i: f"[[{i['page']}]] `{i['field']}:` names `{i['target']}`, which is not a page", + ) + _section( + lines, "Invalid Type Paths", report["invalid_type_paths"], + lambda i: f"[[{i['page']}]] has type: `{i['type']}` - {i['error']}", + ) + _section( + lines, "Type Resolution Errors", report["type_resolution_errors"], + lambda i: f"[[{i['page']}]] type: `{i['type']}` - {i['error']}", + ) + _section( + lines, "Schema Validation Errors", report["schema_validation_errors"], + lambda i: f"[[{i['page']}]] type: `{i['type']}` - {i['error']}", + ) + _section( + lines, f"Pages Exceeding Quote Limit (>{QUOTE_LIMIT}/page)", report["quote_limit_violations"], + lambda i: f"[[{i['page']}]] has {i['quote_count']} quotes - trim or confirm they're load-bearing", + ) + + lines.append("## Semantic Review (LLM to complete)") + lines.append("") + lines.append("- Contradictions across pages: TODO") + lines.append("- Stale claims (unconfirmed >6 months): TODO") + lines.append("- Suggested new pages / missing cross-references: TODO") + lines.append("") + return "\n".join(lines) + + +# Sections that always carry content but are not findings, so the summary +# handles them separately: a hub list is a statistic, and the semantic review +# is the checklist that follows the report rather than part of it. +INFORMATIONAL_SECTIONS = ("Most-Linked Pages",) +SEMANTIC_REVIEW_SECTION = "Semantic Review" + + +def _split_sections(markdown: str) -> tuple[str, list[tuple[str, str]]]: + """Cut a rendered report into its preamble and (title, body) sections.""" + preamble, *rest = markdown.split("\n## ") + sections = [] + for part in rest: + title, _, body = part.partition("\n") + sections.append((title.strip(), body.strip())) + return preamble.rstrip(), sections + + +def render_summary(report: dict) -> str: + """The same report with the empty sections removed. + + On a healthy corpus the full report is better than 90% "None found.", so + reading it in the terminal means paging past the answer. The file on disk + stays complete - this is what gets printed, and the written path underneath + it is how the rest is reached without running lint a second time. + """ + preamble, sections = _split_sections(render_markdown(report)) + findings, trailing = [], [] + for title, body in sections: + if title.startswith(SEMANTIC_REVIEW_SECTION): + trailing.append((title, body)) + elif body != "None found." and not title.startswith(INFORMATIONAL_SECTIONS): + findings.append((title, body)) + lines = [preamble, ""] + if not findings: + lines += ["No structural findings.", ""] + for title, body in findings + trailing: + lines += [f"## {title}", "", body, ""] + return "\n".join(lines) + + +def default_report_path(report: dict) -> Path: + """Where a report goes when the caller names no path. + + `reports/` is derived and gitignored ([reports/CONTRACT.md]), so writing + here by default costs the tree nothing. + """ + return config.ROOT / "reports" / f"Lint Report {report['generated']}.md" + + +# Findings that make a tree structurally wrong rather than merely untidy. +# `orphan_pages` is deliberately absent: many pages are validly reachable +# through the index or navigation only. `quote_limit_violations` is advisory +# too - it flags a habit, not a broken tree. +# +# One definition, used by `lint --fail-on-error` and by the eval scorecard: if +# the two disagreed, a run could pass its score while lint refused it. +HARD_ERROR_KEYS = ( + "frontmatter_errors", + "broken_links", + "dangling_index_entries", + "duplicate_titles", + "broken_raw_refs", + "duplicate_raw_file_owners", + "legacy_source_pages", + "citation_frontmatter_drift", + "legacy_citation_markers", + "undefined_footnote_refs", + "orphan_footnote_defs", + "dangling_frontmatter_refs", + "invalid_type_paths", + "type_resolution_errors", + "schema_validation_errors", +) + + +def has_hard_errors(report: dict) -> bool: + return any(report.get(key) for key in HARD_ERROR_KEYS) diff --git a/tools/chemenu/mcp/__init__.py b/tools/chemenu/mcp/__init__.py new file mode 100644 index 0000000..ec5d5e8 --- /dev/null +++ b/tools/chemenu/mcp/__init__.py @@ -0,0 +1,8 @@ +"""The MCP read server: Chemenu's second consumer. + +The CLI and this server are two adapters over one core (`chemenu.api`), not a +CLI with a network interface bolted on. See `server.py`. +""" +from chemenu.mcp.server import build_server + +__all__ = ["build_server"] diff --git a/tools/chemenu/mcp/__main__.py b/tools/chemenu/mcp/__main__.py new file mode 100644 index 0000000..1075df3 --- /dev/null +++ b/tools/chemenu/mcp/__main__.py @@ -0,0 +1,79 @@ +"""`python -m chemenu.mcp` - start the read server. + +Deliberately argparse and not typer. This process is the one place that must +not pull the CLI head in: the whole point of the library boundary is that a +second consumer costs `yaml`, `jsonschema` and the MCP SDK, and nothing else. + + python -m chemenu.mcp # stdio + python -m chemenu.mcp --transport streamable-http # behind the proxy + CHEMENU_ROOT=/srv/wiki python -m chemenu.mcp --transport streamable-http \ + --host 0.0.0.0 --port 8000 + +`--host`/`--port` apply to `streamable-http` only. They are here because the +default binds loopback, and a server in a container with a reverse proxy in +front of it has to bind an interface the proxy can reach - that is a property +of the software, not of one installation. *Which* host and port a given +deployment picks is infrastructure and stays out of this repository. +""" +from __future__ import annotations + +import argparse +import sys + +from chemenu.mcp.server import TRANSPORTS, TraceWouldWriteIntoCorpus, serve + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="python -m chemenu.mcp", + description="Serve a Chemenu wiki read-only over MCP.", + ) + parser.add_argument( + "--transport", + choices=TRANSPORTS, + default="stdio", + help="stdio for local use and testing; streamable-http for a deployed " + "instance behind the Traefik middleware (default: stdio)", + ) + parser.add_argument( + "--root", + default=None, + help="The corpus to serve. Defaults to $CHEMENU_ROOT, then the checkout " + "this package lives in.", + ) + parser.add_argument( + "--host", + default="127.0.0.1", + help="Interface to bind, streamable-http only. The loopback default is " + "deliberate; a container behind a reverse proxy needs 0.0.0.0.", + ) + parser.add_argument( + "--port", + type=int, + default=8000, + help="Port to bind, streamable-http only (default: 8000)", + ) + args = parser.parse_args(argv) + + # Passed only for the transport they mean anything to: `run_stdio_async` + # takes no host or port, and handing it one is a TypeError rather than a + # harmless no-op. + bind = ( + {"host": args.host, "port": args.port} + if args.transport == "streamable-http" + else {} + ) + + try: + serve(transport=args.transport, root=args.root, **bind) + except TraceWouldWriteIntoCorpus as exc: + # Refused before binding anything: the message names both fixes, and a + # server that silently relocated the operator's telemetry instead would + # be a surprise buried in a log. + print(f"ERROR {exc}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": # pragma: no cover - process entry point + raise SystemExit(main()) diff --git a/tools/chemenu/mcp/server.py b/tools/chemenu/mcp/server.py new file mode 100644 index 0000000..303ea4c --- /dev/null +++ b/tools/chemenu/mcp/server.py @@ -0,0 +1,234 @@ +"""The MCP read server: `search`, `types`, `lint` and `status` over `kb/`. + +Chemenu's second consumer. The CLI and this are two adapters over one core - +`chemenu.api.Corpus` - so a question answered here and the same question asked +at a terminal go through the same code, and a golden test holds the two +outputs against each other rather than trusting that they agree. + +**There is no write path, structurally.** Nothing under `chemenu.commands` is +imported here or in `chemenu.api`, so `new`, `touch`, `xref`, `cite`, +`publish`, `migrate` and `version bump` are not reachable - the functions do +not exist in this process's reach, rather than being filtered out of a list. A +test asserts it by importing this module in a clean interpreter and looking at +`sys.modules`. + +**Authentication and rate limiting are not here.** Both are Traefik middleware +in front of the process, per the operator's decision of 2026-09-01: a request +that is not cleanly authenticated does not reach Python at all. What *is* here +is the resource protection that middleware cannot give - the search timeout and +the frontmatter limits - because those exist against an authenticated consumer +damaging itself, which is a different problem from an unauthenticated one. + +**The Iteration Budget Gate is deliberately absent.** It exists to stop an +agent *session* from iterating unnoticed over the state of the wiki, which is +why retrieval is exempt from it in the first place. A user who searches too +often is a resource problem - different instrument, different purpose - and +using the gate as a rate limiter would dilute it into one. + +**Every response carries the commit it was computed from.** `chemenu.api` +stamps `commit` and `as_of`; a stale checkout otherwise answers confidently and +wrongly. Keeping the checkout current is a `git fetch && git reset --hard` +poll outside this process - see `instructions/mcp-read-server.md` - which needs +no inbound endpoint and no signature checking. +""" +from __future__ import annotations + +import os +from pathlib import Path +from typing import Any, Optional + +from mcp.server.mcpserver import MCPServer +from mcp.server.mcpserver.exceptions import ToolError + +from chemenu import config +from chemenu.api import Corpus +from chemenu.errors import ChemenuError + +SERVER_NAME = "chemenu" + +# Transports this server will start on. `stdio` is for developing and testing +# it without a network; `streamable-http` is what a deployed instance speaks, +# and the only one the Traefik middleware can sit in front of, because Traefik +# is an HTTP reverse proxy. `sse` is reachable through the SDK but not offered: +# it is the superseded remote transport, and building on it now only moves the +# migration later. +TRANSPORTS = ("stdio", "streamable-http") + + +class TraceWouldWriteIntoCorpus(RuntimeError): + """Raised at startup when telemetry would land inside the served tree.""" + + +def check_trace_destination(root: Path) -> None: + """Refuse to start if a trace would be written into the corpus. + + Telemetry defaults to *on* and writes under `reports/telemetry/` in the + repo. Today nothing on this path emits - the writer is wired into + `cli.main()` and the two gates, none of which run here - so this is a guard + against the future rather than a fix for the present. It is worth having + anyway: the sync that keeps this checkout current is `git reset --hard`, so + a trace written into the tree is both a per-request write into a directory + something else is entitled to wipe, and a silent way for the server to + dirty the tree its own cache keys on. + + Turn tracing off (`WIKI_TRACE=0`) or point it somewhere else + (`WIKI_TRACE_DIR`). Refusing rather than correcting it: a server that + quietly relocates the operator's telemetry is a surprise waiting in a log + nobody reads. + """ + if os.environ.get("WIKI_TRACE", "1") == "0": + return + destination = os.environ.get("WIKI_TRACE_DIR") + if destination is None: + raise TraceWouldWriteIntoCorpus( + "Telemetry is on and would write into the served checkout " + f"({root / 'reports' / 'telemetry'}). The sync that keeps this checkout " + "current is `git reset --hard`, which is entitled to wipe that directory. " + "Set WIKI_TRACE=0, or point WIKI_TRACE_DIR outside the corpus." + ) + resolved = Path(destination).expanduser().resolve() + try: + resolved.relative_to(Path(root).resolve()) + except ValueError: + return + raise TraceWouldWriteIntoCorpus( + f"WIKI_TRACE_DIR ({resolved}) is inside the served checkout ({root}). " + "Point it outside, or set WIKI_TRACE=0." + ) + + +def build_server( + root: Optional[Path | str] = None, check_trace: bool = True +) -> MCPServer: + """Assemble the server over one corpus. + + `root` follows `config.resolve_root()` - argument, then `$CHEMENU_ROOT`, + then the checkout the package lives in - so a deployment points at its + corpus with one environment variable and no code. + """ + corpus = Corpus(root) + if check_trace: + check_trace_destination(corpus.root) + + server = MCPServer( + name=SERVER_NAME, + instructions=( + "Read access to a Chemenu wiki: compiled, sourced knowledge under kb/. " + "Every answer carries the commit it was computed from ('commit') and " + "when it was produced ('as_of'); a null commit means the served tree " + "has uncommitted changes and the answer corresponds to no revision. " + "This server is read-only - there is no tool that writes." + ), + ) + + @server.tool( + name="search", + description=( + "Find pages in kb/ by text, by frontmatter, or by both. Returns " + "title, path, kind, summary and confidence per hit, so a result can " + "be judged without fetching the page. Prefer this over listing " + "files: the answer is a few hundred tokens instead of a whole index." + ), + ) + def search( + query: str | None = None, + predicates: list[str] | None = None, + regex: bool = False, + limit: int = 20, + sort: str | None = None, + ) -> dict[str, Any]: + """Search the wiki. + + `predicates` are frontmatter filters in the CLI's own `--field` syntax, + ANDed: `confidence<0.6`, `entity_type=system`, `tags~k8s`, `source_url:*` + (present), `!source_url` (absent). With no `query` this is a pure + structured query over frontmatter. + + `regex` applies the pattern with ripgrep's linear engine. It is off by + default, so an accidental `.*` is a literal. + """ + return _guard( + lambda: corpus.search( + text=query, + predicates=predicates or (), + regex=regex, + limit=limit, + sort=sort, + ) + ) + + @server.tool( + name="types", + description=( + "List the page types this wiki declares - what kinds of page exist, " + "where each lives, and what its schema is. Read this before " + "interpreting a page's `kind`." + ), + ) + def types() -> dict[str, Any]: + return _guard(corpus.types) + + @server.tool( + name="describe_type", + description=( + "One page type's full contract: its frontmatter fields with " + "required/optional and any enums, its subtype field, and its " + "authoring guidance." + ), + ) + def describe_type(name: str) -> dict[str, Any]: + """`name` is a short type name as listed by `types`, e.g. 'entity'.""" + return _guard(lambda: corpus.describe_type(name)) + + @server.tool( + name="lint", + description=( + "The wiki's structural health: broken wikilinks, orphan pages, " + "index drift, schema gaps, provenance gaps. Findings only - the " + "JSON form writes no report file." + ), + ) + def lint() -> dict[str, Any]: + return _guard(corpus.lint) + + @server.tool( + name="status", + description=( + "A snapshot: how many pages the wiki holds, how they split across " + "collections, and how many findings of each kind lint reports. " + "Cheaper to read than the full lint output." + ), + ) + def status() -> dict[str, Any]: + return _guard(corpus.status) + + return server + + +def _guard(call): + """Turn a `ChemenuError` into a plain message for the protocol layer. + + A bad predicate is the caller's argument, not a server fault, and it should + arrive as a tool error the model can act on, carrying the message that says + what to do differently. The SDK draws exactly this line: a `ToolError` is a + deliberate refusal and its text reaches the caller, while anything else is a + crash whose text stays on the server. Only `ChemenuError` is caught - + everything else is a genuine fault and belongs in the log, unswallowed. + """ + try: + return call() + except ChemenuError as exc: + raise ToolError(str(exc)) from exc + + +def serve( + transport: str = "stdio", + root: Optional[Path | str] = None, + **kwargs: Any, +) -> None: + """Start the server. `transport` is one of `TRANSPORTS`.""" + if transport not in TRANSPORTS: + raise ValueError( + f"unknown transport {transport!r}. Available: {', '.join(TRANSPORTS)}" + ) + build_server(root).run(transport=transport, **kwargs) diff --git a/tools/chemenu/page.py b/tools/chemenu/page.py index 2fefcdc..bdd0452 100644 --- a/tools/chemenu/page.py +++ b/tools/chemenu/page.py @@ -17,6 +17,14 @@ class Page: frontmatter: dict[str, Any] = field(default_factory=dict) body: str = "" + # Why the frontmatter above is empty, when it is empty for a reason. A page + # whose YAML does not parse reads back as `{}`, and a `{}` page has no + # `confidence` and no `kind`: it then drops out of `--field confidence<0.6` + # - the query whose whole purpose is to find pages in bad shape - looking + # exactly like a page that did not match. Loaders that know the reason put + # it here so a caller can report the page instead of losing it. + frontmatter_error: Optional[str] = None + @property def title(self) -> str: """The page's canonical title: the filename without extension. diff --git a/tools/chemenu/search/filters.py b/tools/chemenu/search/filters.py index 4efdad6..432b631 100644 --- a/tools/chemenu/search/filters.py +++ b/tools/chemenu/search/filters.py @@ -14,6 +14,7 @@ from pathlib import Path from typing import Any, Optional from chemenu import config +from chemenu.errors import ValidationError from chemenu.page import Page from chemenu.search.types import Predicate @@ -26,7 +27,7 @@ _COMPARISON_OPS = (">=", "<=", ">", "<", "~", "=") VIRTUAL_FIELDS = ("title", "kind", "subtype", "collection") -class PredicateError(ValueError): +class PredicateError(ValidationError): """Raised for a malformed `--field` argument or an unknown field name.""" diff --git a/tools/chemenu/search/registry.py b/tools/chemenu/search/registry.py index cce4dce..7348713 100644 --- a/tools/chemenu/search/registry.py +++ b/tools/chemenu/search/registry.py @@ -7,28 +7,39 @@ the output shape. Selecting several at once fuses them through RRF. from __future__ import annotations import os -from typing import Callable +from pathlib import Path +from typing import Callable, Optional +from chemenu.errors import ValidationError from chemenu.search.base import SearchBackend from chemenu.search.ripgrep import RipgrepBackend DEFAULT_BACKEND = "rg" ENV_VAR = "WIKITOOL_SEARCH_BACKEND" -BACKENDS: dict[str, Callable[[], SearchBackend]] = { +BACKENDS: dict[str, Callable[..., SearchBackend]] = { "rg": RipgrepBackend, } -class UnknownBackend(ValueError): +class UnknownBackend(ValidationError): pass -def resolve(spec: str | None = None) -> list[SearchBackend]: +def resolve( + spec: str | None = None, + kb_dir: Optional[Path] = None, + root: Optional[Path] = None, +) -> list[SearchBackend]: """Resolve a backend spec into instances. Precedence: explicit argument, then `WIKITOOL_SEARCH_BACKEND`, then the default. A comma-separated spec selects several and fuses their rankings. + + `kb_dir`/`root` are handed to the backend rather than left to its own + defaults. Without them a caller could pass a corpus to `run_search` and + still have the backend walk `config.KB_DIR` - the query answered from one + tree and the pages read from another, with nothing saying so. """ raw = spec or os.environ.get(ENV_VAR) or DEFAULT_BACKEND names = [n.strip() for n in raw.split(",") if n.strip()] @@ -38,4 +49,4 @@ def resolve(spec: str | None = None) -> list[SearchBackend]: f"unknown search backend(s): {', '.join(unknown)}. " f"Available: {', '.join(sorted(BACKENDS))}" ) - return [BACKENDS[name]() for name in names] + return [BACKENDS[name](kb_dir, root) for name in names] diff --git a/tools/chemenu/search/ripgrep.py b/tools/chemenu/search/ripgrep.py index b568bcb..dfb5035 100644 --- a/tools/chemenu/search/ripgrep.py +++ b/tools/chemenu/search/ripgrep.py @@ -4,7 +4,7 @@ Why shell out instead of scanning in Python: `rg` is already the retrieval layer the agent instructions point at, it handles large trees fast, and its `--json` mode gives line numbers and matched text without reparsing files. -Two safety properties are load-bearing and must survive any edit here: +Three safety properties are load-bearing and must survive any edit here: 1. The query is passed as an *argv element*, never through a shell. There is no `shell=True` anywhere in this module, so a query containing `;`, `$(...)` @@ -12,16 +12,19 @@ Two safety properties are load-bearing and must survive any edit here: 2. `--fixed-strings` is the default. A user-supplied regex is opt-in via `--regex`, so an accidental `.*` in a search term is a literal, and a pathological pattern cannot be introduced without asking for one. +3. A user-supplied pattern is evaluated **only** by `rg`, whose engine is + linear in the input. Nothing here hands it to Python's `re`, which + backtracks - see `_contains`. """ from __future__ import annotations import json -import re import subprocess from pathlib import Path from typing import Iterable from chemenu import config +from chemenu.errors import BackendError from chemenu.page import Page from chemenu.search.base import page_key from chemenu.search.filters import collection_of @@ -41,13 +44,22 @@ TITLE_WEIGHT = 5.0 SUMMARY_WEIGHT = 3.0 LINE_WEIGHT = 1.0 +# A hang-breaker, not a performance budget. A fixed-string search over this +# corpus costs 6 ms and a deliberately broad regex 1.4 s, so nothing legitimate +# comes near this; it exists so that a pathological pattern, a corpus on a +# stalled network mount, or an `rg` that never returns fails as an error +# instead of holding the caller open forever while its output buffers into the +# heap. The caller sees the ordinary `RipgrepFailed` path. +RIPGREP_TIMEOUT_SECONDS = 30.0 -class RipgrepMissing(RuntimeError): + +class RipgrepMissing(BackendError): """Raised when the `rg` executable is not on PATH.""" -class RipgrepFailed(RuntimeError): - """Raised when `rg` exits with an error (exit code 2 or above).""" +class RipgrepFailed(BackendError): + """Raised when `rg` exits with an error (exit code 2 or above), or had to + be killed for running past `RIPGREP_TIMEOUT_SECONDS`.""" def build_argv(query: SearchQuery, root: Path) -> list[str]: @@ -99,7 +111,20 @@ class RipgrepBackend: argv = build_argv(query, self.search_root) try: - proc = subprocess.run(argv, capture_output=True, text=True, check=False) + proc = subprocess.run( + argv, + capture_output=True, + text=True, + check=False, + timeout=RIPGREP_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired as exc: + raise RipgrepFailed( + f"rg did not finish within {RIPGREP_TIMEOUT_SECONDS:g}s and was killed. " + "A search that takes this long is a pathological pattern or an " + "unresponsive corpus directory, not a slow answer - narrow the query " + "or use a fixed string instead of --regex." + ) from exc except FileNotFoundError as exc: # pragma: no cover - depends on host raise RipgrepMissing( "ripgrep (rg) is not installed or not on PATH. It is the search " @@ -141,13 +166,26 @@ class RipgrepBackend: def _contains(haystack: str, query: SearchQuery) -> bool: + """Literal containment, used only for the title and summary ranking boosts. + + It never evaluates `query.text` as a regex, even when `query.regex` is set. + It used to, via `re.search`, and Python's engine backtracks: `(\\w+\\s?)+$` + against 114 characters of ordinary page text does not terminate in eight + seconds, while a pattern that fails deterministically takes 0.2 ms - the + difference is the pattern, not the haystack. `build_hit` calls this twice + per hit, and a pattern as cheap as `\\w` matches every page, so one request + bought two unbounded searches per page in the corpus. + + Deleting the branch rather than bounding it is the right trade: `rg` has + already applied the pattern with a linear engine by the time we get here, + and the page is a hit *because* of that. What is lost is only the extra + weight a regex hit in the title would have scored - and since a summary and + an H1 are themselves lines in the file, `rg` still counts them. A pattern + that is mostly literal (`longhorn`) still earns its boost through the test + below; one that is not gets ranked by match count alone. + """ if not query.text: return False - if query.regex: - try: - return re.search(query.text, haystack, re.IGNORECASE) is not None - except re.error: - return False return query.text.lower() in haystack.lower() diff --git a/tools/chemenu/search/service.py b/tools/chemenu/search/service.py new file mode 100644 index 0000000..d0a8382 --- /dev/null +++ b/tools/chemenu/search/service.py @@ -0,0 +1,112 @@ +"""The search core, with no CLI attached. + +`run_search()` and the corpus loader used to live in `commands/search.py`, +which imports `typer` at module level and `rich` through `_util`. Any +in-process caller therefore dragged the whole CLI head in behind it - which +made the "two third-party packages" the read core actually needs (`yaml`, +`jsonschema`) an accounting fiction rather than a fact about the import graph. + +Nothing here imports `typer`, `rich`, or anything under `chemenu.commands`. +That is the boundary, and it is worth keeping: `commands/search.py` is now the +adapter that turns these values into terminal output and these exceptions into +exit codes, and the MCP server (Gitea #19) is a second adapter over the same +functions rather than a second implementation of them. + +Errors are raised, never printed: `PredicateError` for a bad `--field`, +`RipgrepMissing`/`RipgrepFailed` for the backend. All of them are +`ChemenuError` - see `chemenu/errors.py`. +""" +from __future__ import annotations + +from pathlib import Path + +from chemenu import config +from chemenu.frontmatter_io import read_page_with_error +from chemenu.kb_scan import iter_kb_pages +from chemenu.page import Page +from chemenu.search import filters +from chemenu.search.base import page_key +from chemenu.search.fuse import reciprocal_rank_fusion +from chemenu.search.ripgrep import build_hit +from chemenu.search.types import SearchHit, SearchQuery + + +def load_pages_by_path(kb_dir: Path | None = None, root: Path | None = None) -> dict[str, Page]: + """Every page under `kb/`, keyed by repo-relative path. + + Path-keyed rather than title-keyed on purpose: `load_kb_pages()` drops one + of two pages sharing a stem, and search should still find both - a + duplicate title is a lint finding, not a reason to hide a page. + """ + kb_dir = kb_dir or config.KB_DIR + root = root or config.ROOT + pages: dict[str, Page] = {} + for path in iter_kb_pages(kb_dir): + frontmatter, body, error = read_page_with_error(path) + pages[page_key(path, root)] = Page( + path=path, frontmatter=frontmatter, body=body, frontmatter_error=error + ) + return pages + + +def unreadable_pages(pages: dict[str, Page]) -> list[dict[str, str]]: + """The pages whose frontmatter could not be used, as `{path, reason}`. + + Reported rather than swallowed. Such a page has no `confidence` and no + `kind`, so it silently drops out of every positive `--field` predicate - + including the low-confidence sweep that exists to find pages in exactly + that state. Saying nothing makes it look like a page that did not match; + an empty block is excluded, because a page can legitimately carry one. + """ + return [ + {"path": key, "reason": page.frontmatter_error} + for key, page in sorted(pages.items()) + if page.frontmatter_error and page.frontmatter_error != "empty frontmatter block" + ] + + +def _sort_key(hit: SearchHit, field: str): + value = hit.as_dict().get(field) + if value is None: + # Missing values sort last in either direction rather than crashing on + # a None comparison. + return (1, "") + if isinstance(value, (int, float)): + return (0, value) + return (0, str(value).lower()) + + +def sort_hits(hits: list[SearchHit], sort: str | None) -> list[SearchHit]: + """Sort by a hit field. A leading `-` reverses, e.g. `--sort -confidence`.""" + if not sort: + return hits + descending = sort.startswith("-") + field = sort.lstrip("-") + ordered = sorted(hits, key=lambda h: _sort_key(h, field), reverse=descending) + return ordered + + +def run_search( + query: SearchQuery, + pages: dict[str, Page], + backends: list, + kb_dir: Path | None = None, +) -> list[SearchHit]: + """Answer a query. Pure: no I/O beyond whatever a backend does.""" + filters.validate_fields(query.predicates, pages) + + if query.text: + rankings = [backend.search(query, pages) for backend in backends] + hits = rankings[0] if len(rankings) == 1 else reciprocal_rank_fusion(rankings) + allowed = filters.apply_predicates(pages, query.predicates, kb_dir) + hits = [hit for hit in hits if hit.path in allowed] + else: + selected = filters.apply_predicates(pages, query.predicates, kb_dir) + hits = [ + build_hit(page, key, [], query, backend="frontmatter", kb_dir=kb_dir) + for key, page in selected.items() + ] + hits.sort(key=lambda h: h.title.lower()) + + hits = sort_hits(hits, query.sort) + return hits[: query.limit] if query.limit else hits diff --git a/tools/chemenu/tests/conftest.py b/tools/chemenu/tests/conftest.py index ebd85f1..4b80f20 100644 --- a/tools/chemenu/tests/conftest.py +++ b/tools/chemenu/tests/conftest.py @@ -5,6 +5,7 @@ import pytest from chemenu import config from chemenu.frontmatter_io import write_page +from chemenu.type_resolver import resolver # Environment the tool reads for its own behaviour. Cleared for every test, so # that a test which needs one sets it itself and the rest run against the @@ -18,6 +19,7 @@ _WIKITOOL_ENV = ( "WIKITOOL_SESSION_ID", "WIKITOOL_UPDATE_URL", "WIKITOOL_UPDATE_TOKEN", + "CHEMENU_ROOT", ) # Environment git reads for identity or for where its repo lives. A stray @@ -36,7 +38,7 @@ _GIT_ENV = ( @pytest.fixture(autouse=True) -def hermetic_environment(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: +def hermetic_environment(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): """Cut every test off from the machine it runs on. The suite was green for months while silently depending on whoever ran it: @@ -68,7 +70,34 @@ def hermetic_environment(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Pat for name in (*_WIKITOOL_ENV, *_GIT_ENV): monkeypatch.delenv(name, raising=False) - return home + + # The same hole as the environment above, one layer in: `config` resolves + # its paths on access, and `monkeypatch.setattr(config, "KB_DIR", ...)` + # undoes itself by writing the *resolved* old path back as a real + # attribute. That binding outlives the test and hands the next one a + # corpus directory belonging to the previous tree. Cleared on both sides, + # so neither a leak from before nor one from this test can be inherited. + config.reset() + yield home + config.reset() + + +def use_shipped_type_specs(monkeypatch: pytest.MonkeyPatch) -> None: + """Keep the shipped `types/` reachable for a test that repoints `ROOT`. + + `TYPES_DIR` and the `TypeResolver`'s own root both follow `ROOT` now, which + is the whole point of making the resolution lazy - but it means a fixture + tree without a `types/` resolves no page kind at all, and a check that + depends on a page being a `source` silently stops finding one. These tests + do want the real schema: a synthetic type-spec would prove the command + works against a fixture rather than against what it ships with. + + So the dependency is declared instead of inherited. While both were bound at + import time it held by accident, which is the same shape as the hole + `raw_dir` was written to close, one layer down. + """ + monkeypatch.setattr(config, "TYPES_DIR", config._PACKAGE_ROOT / "types") + monkeypatch.setattr(resolver, "_repo_root", config._PACKAGE_ROOT) @pytest.fixture(autouse=True) @@ -113,6 +142,7 @@ def raw_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: same way - in the fixture, not in the one test that happened to trip. """ monkeypatch.setattr(config, "ROOT", tmp_path) + use_shipped_type_specs(monkeypatch) raw = tmp_path / "raw" (raw / "notes").mkdir(parents=True) (raw / "notes" / "Aurora.md").write_text("# Aurora raw notes\n", encoding="utf-8") diff --git a/tools/chemenu/tests/test_api.py b/tools/chemenu/tests/test_api.py new file mode 100644 index 0000000..59d1eeb --- /dev/null +++ b/tools/chemenu/tests/test_api.py @@ -0,0 +1,184 @@ +"""Tests for the in-process library boundary (Gitea #31). + +Two properties, and neither is about the values coming back: + +1. A caller can point Chemenu at a corpus tree and **no path of the checkout + this package lives in is read**. That is what "library" means here, and it + is what could not be asserted before: `config.KB_DIR` was bound at import + time, so a caller repointing `ROOT` was still answered out of the developer's + own `kb/`. +2. The surface is read-only **structurally**. `chemenu.api` imports nothing + under `chemenu.commands`, so `new`, `publish` and the rest are not reachable + from it - not filtered out of it. +""" +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +import pytest + +from chemenu import config +from chemenu.api import Corpus +from chemenu.errors import ChemenuError, ValidationError +from chemenu.types_core import UnknownType + + +@pytest.fixture +def foreign_corpus(tmp_path: Path) -> Path: + """A corpus tree that is not this checkout, with one findable page.""" + root = tmp_path / "elsewhere" + kb = root / "kb" / "entities" + kb.mkdir(parents=True) + (root / "kb" / "entities" / "COLLECTION.md").write_text("# entities\n", encoding="utf-8") + (kb / "Peregrine.md").write_text( + "---\ntype: types/entity.md\nentity_type: system\nconfidence: 0.42\n" + "summary: A system that exists only in this fixture.\n---\n\n" + "# Peregrine\n\nPeregrine is the fixture's own system.\n", + encoding="utf-8", + ) + return root + + +def test_a_corpus_can_be_named_and_is_the_one_that_answers(foreign_corpus): + result = Corpus(foreign_corpus).search("Peregrine") + assert [hit["title"] for hit in result["results"]] == ["Peregrine"] + assert result["results"][0]["path"] == "kb/entities/Peregrine.md" + + +def test_no_path_of_this_checkout_is_read_while_a_foreign_root_is_set(foreign_corpus): + """The acceptance criterion, asserted rather than argued. + + `Path.read_text` and `Path.rglob` are the two ways a page reaches the + reader; both are watched, and any access under the real repository root + fails the test. Before the root resolution was made lazy this test could not + pass: `config.KB_DIR` was already bound to this checkout's `kb/`. + """ + package_root = config._PACKAGE_ROOT + trespasses: list[str] = [] + + real_read_text = Path.read_text + real_rglob = Path.rglob + + def watched_read_text(self, *args, **kwargs): + _note(self) + return real_read_text(self, *args, **kwargs) + + def watched_rglob(self, *args, **kwargs): + _note(self) + return real_rglob(self, *args, **kwargs) + + def _note(path: Path) -> None: + try: + path.resolve().relative_to(package_root) + except ValueError: + return + trespasses.append(str(path)) + + monkey = pytest.MonkeyPatch() + monkey.setattr(Path, "read_text", watched_read_text) + monkey.setattr(Path, "rglob", watched_rglob) + try: + Corpus(foreign_corpus).search("Peregrine") + Corpus(foreign_corpus).search(predicates=["confidence<0.6"]) + finally: + monkey.undo() + + assert trespasses == [] + + +def test_the_env_var_points_the_default_corpus(foreign_corpus, monkeypatch): + """`CHEMENU_ROOT` exists for a caller that *is* a whole process and has + nothing to pass an argument through.""" + monkeypatch.setenv(config.ENV_ROOT, str(foreign_corpus)) + assert Corpus().root == foreign_corpus.resolve() + assert Corpus().kb_dir == foreign_corpus.resolve() / "kb" + + +def test_without_the_env_var_the_root_is_this_checkout(monkeypatch): + """The default has to be unchanged, or `tools/wikitool` moves under + everyone's feet.""" + monkeypatch.delenv(config.ENV_ROOT, raising=False) + assert config.resolve_root() == config._PACKAGE_ROOT + + +def test_an_explicit_argument_beats_the_env_var(foreign_corpus, tmp_path, monkeypatch): + """A caller serving two corpora cannot tell them apart with a process-wide + variable, so the argument has to win.""" + monkeypatch.setenv(config.ENV_ROOT, str(tmp_path / "somewhere-else")) + assert Corpus(foreign_corpus).root == foreign_corpus.resolve() + + +def test_derived_paths_follow_the_root_instead_of_lagging_behind(foreign_corpus, monkeypatch): + """The failure that made the old shape worse than the limitation: `ROOT` + moved and `KB_DIR` did not, so a caller believed it was working on the + target tree while reading this one.""" + monkeypatch.setattr(config, "ROOT", foreign_corpus) + assert config.KB_DIR == foreign_corpus / "kb" + assert config.RAW_DIR == foreign_corpus / "raw" + assert config.INDEX_FILE == foreign_corpus / "kb" / "index.md" + + +def test_validation_errors_are_raised_not_exited(foreign_corpus): + corpus = Corpus(foreign_corpus) + with pytest.raises(ValidationError): + corpus.search() + with pytest.raises(ValidationError): + corpus.search("x", predicates=["not a predicate"]) + with pytest.raises(UnknownType): + corpus.describe_type("no-such-type") + # One class to catch, whatever went wrong. + with pytest.raises(ChemenuError): + corpus.describe_type("no-such-type") + + +def test_the_read_surface_cannot_reach_a_write_command(): + """Structural, not filtered: import `chemenu.api` in a clean interpreter and + nothing under `chemenu.commands` is loaded, so there is no `publish` to + call. Run out-of-process because this suite has already imported the CLI.""" + code = ( + "import sys, chemenu.api;" + "print([m for m in sys.modules if m.startswith('chemenu.commands')]);" + "print([m for m in sys.modules if m in ('typer', 'rich', 'click')])" + ) + result = subprocess.run( + [sys.executable, "-c", code], + cwd=config._PACKAGE_ROOT / "tools", + capture_output=True, + text=True, + check=True, + ) + commands_loaded, cli_loaded = result.stdout.strip().splitlines() + assert commands_loaded == "[]", f"api pulled in command modules: {commands_loaded}" + assert cli_loaded == "[]", f"api pulled in the CLI head: {cli_loaded}" + + +def test_every_answer_carries_the_revision_it_was_computed_from(foreign_corpus): + """A stale checkout answers confidently and wrongly otherwise. Outside git + there is no commit, and the stamp says so rather than inventing one.""" + corpus = Corpus(foreign_corpus) + result = corpus.search("Peregrine") + assert result["commit"] is None and result["as_of"] + + subprocess.run(["git", "init", "-b", "main"], cwd=foreign_corpus, check=True, + capture_output=True) + for key, value in (("user.name", "Fixture"), ("user.email", "f@example.com")): + subprocess.run(["git", "config", key, value], cwd=foreign_corpus, check=True, + capture_output=True) + subprocess.run(["git", "add", "-A"], cwd=foreign_corpus, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "corpus"], cwd=foreign_corpus, check=True, + capture_output=True) + + stamped = Corpus(foreign_corpus).search("Peregrine") + head = subprocess.run(["git", "rev-parse", "HEAD"], cwd=foreign_corpus, + capture_output=True, text=True, check=True).stdout.strip() + assert stamped["commit"] == head + + +def test_lint_and_status_answer_from_the_named_corpus(foreign_corpus): + corpus = Corpus(foreign_corpus) + assert corpus.lint()["page_count"] == 1 + status = corpus.status() + assert status["pages"] == 1 + assert status["collections"] == {"entities": 1} diff --git a/tools/chemenu/tests/test_corpus_cache.py b/tools/chemenu/tests/test_corpus_cache.py new file mode 100644 index 0000000..c9db233 --- /dev/null +++ b/tools/chemenu/tests/test_corpus_cache.py @@ -0,0 +1,98 @@ +"""Tests for the revision-keyed corpus cache (Gitea #33, feeding #19). + +The property under test is not "it is fast" but "it is never stale". A cache on +this path that answers from a superseded parse is worse than the reparse it +replaces: the caller gets a confident answer about a corpus that no longer +exists, which is the failure `SOUL.md` names as the cardinal one. +""" +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from chemenu import config, corpus_cache +from chemenu.corpus_cache import CorpusCache + + +def _git(root: Path, *args: str) -> None: + subprocess.run(["git", *args], cwd=root, check=True, capture_output=True) + + +@pytest.fixture +def repo(kb_dir: Path, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + """The `kb_dir` fixture tree, committed - so there is a revision to key on.""" + _git(tmp_path, "init", "-b", "main") + _git(tmp_path, "config", "user.name", "Fixture Author") + _git(tmp_path, "config", "user.email", "fixture@example.com") + _git(tmp_path, "add", "-A") + _git(tmp_path, "commit", "-m", "fixture corpus") + monkeypatch.setattr(config, "ROOT", tmp_path) + monkeypatch.setattr(config, "KB_DIR", kb_dir) + return tmp_path + + +def test_a_clean_tree_is_cached_and_reused(repo, kb_dir): + cache = CorpusCache(kb_dir, repo) + first, revision = cache.load() + second, again = cache.load() + assert revision is not None and again == revision + # Identity, not equality: an equal-but-rebuilt dict would mean the reparse + # still happened, which is the whole cost this exists to remove. + assert first is second + + +def test_a_new_commit_invalidates_the_cache(repo, kb_dir): + cache = CorpusCache(kb_dir, repo) + first, first_revision = cache.load() + + (kb_dir / "entities" / "Latecomer.md").write_text( + "---\ntype: types/entity.md\n---\n\n# Latecomer\n", encoding="utf-8" + ) + _git(repo, "add", "-A") + _git(repo, "commit", "-m", "one more page") + + second, second_revision = cache.load() + assert second_revision != first_revision + assert len(second) == len(first) + 1 + + +def test_a_dirty_tree_is_never_cached(repo, kb_dir): + """The correctness case. A session that writes a page and then searches for + it must not be answered from the parse taken before the write - and nothing + about the commit SHA changed in between.""" + cache = CorpusCache(kb_dir, repo) + first, revision = cache.load() + assert revision is not None + + (kb_dir / "entities" / "Uncommitted.md").write_text( + "---\ntype: types/entity.md\n---\n\n# Uncommitted\n", encoding="utf-8" + ) + + second, dirty_revision = cache.load() + assert dirty_revision is None + assert len(second) == len(first) + 1 + assert cache.revision is None + + +def test_a_tree_outside_git_reloads_every_time(kb_dir, tmp_path): + cache = CorpusCache(kb_dir, tmp_path) + pages, revision = cache.load() + assert revision is None and pages + assert cache.load()[0] is not pages + + +def test_an_unanswerable_git_counts_as_dirty(repo, kb_dir, monkeypatch): + """Errs toward reloading: a cache that reads "unknown" as "clean" serves + stale pages, which is the one outcome this module exists to prevent.""" + monkeypatch.setattr(corpus_cache, "_git", lambda *a, **k: None) + assert corpus_cache.is_dirty(repo, kb_dir) is True + assert CorpusCache(kb_dir, repo).load()[1] is None + + +def test_head_commit_is_the_revision_a_response_gets_stamped_with(repo): + expected = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo, capture_output=True, text=True, check=True + ).stdout.strip() + assert corpus_cache.head_commit(repo) == expected diff --git a/tools/chemenu/tests/test_doctor.py b/tools/chemenu/tests/test_doctor.py index 754d438..356fb44 100644 --- a/tools/chemenu/tests/test_doctor.py +++ b/tools/chemenu/tests/test_doctor.py @@ -220,6 +220,40 @@ def test_a_renamed_but_unfilled_environment_template_warns(instance): assert "template" in next(c.detail for c in checks if c.name == "environment") +def _remotes_detail(checks) -> str: + return next(c.detail for c in checks if c.name == "publish-remotes") + + +def test_publish_remotes_says_not_armed_when_the_file_is_absent(instance): + """Absence stays OK - a single-remote checkout has nothing to protect - but + the line has to say the gate is off. AGENTS.md lists it among the limits + enforced in code, so "no .wikitool-remotes.json" alone leaves a reader + trusting a safeguard that is not running.""" + checks = doctor.run_doctor() + assert _status(checks, "publish-remotes") == "OK" + assert "not armed" in _remotes_detail(checks) + + +def test_publish_remotes_says_armed_when_the_file_lists_a_target(instance): + (config.ROOT / config.PUBLISH_REMOTES_FILENAME).write_text( + '{ "schema": 1, "allowed_push_urls": ["ssh://git@example.net/one.git"] }', + encoding="utf-8", + ) + checks = doctor.run_doctor() + assert _status(checks, "publish-remotes") == "OK" + detail = _remotes_detail(checks) + assert "armed" in detail and "not armed" not in detail + + +def test_publish_remotes_warns_on_several_remotes_without_an_allowlist(instance): + """The shape a private instance has once it adds the public upstream.""" + _git(config.ROOT, "remote", "add", "origin", "ssh://git@example.net/mine.git") + _git(config.ROOT, "remote", "add", "upstream", "ssh://git@example.net/theirs.git") + checks = doctor.run_doctor() + assert _status(checks, "publish-remotes") == "WARN" + assert "not armed" in _remotes_detail(checks) + + def test_missing_generated_file_fails(instance): config.LOG_FILE.unlink() checks = doctor.run_doctor() diff --git a/tools/chemenu/tests/test_frontmatter_io.py b/tools/chemenu/tests/test_frontmatter_io.py index 359c4f7..96ba95f 100644 --- a/tools/chemenu/tests/test_frontmatter_io.py +++ b/tools/chemenu/tests/test_frontmatter_io.py @@ -1,4 +1,17 @@ -from chemenu.frontmatter_io import dump_frontmatter, read_page, write_page +import pytest +import yaml + +from chemenu.frontmatter_io import ( + MAX_FRONTMATTER_BYTES, + FrontmatterError, + _Loader, + dump_frontmatter, + frontmatter_error, + read_page, + read_page_strict, + read_page_with_error, + write_page, +) @@ -82,3 +95,91 @@ def test_scalar_quoting_is_unchanged_by_the_flow_fix(tmp_path): assert dump_frontmatter({"year": "1945"}) == "year: '1945'" assert dump_frontmatter({"summary": "He said hi"}) == "summary: He said hi" assert dump_frontmatter({"confidence": 0.85}) == "confidence: 0.85" + + +# --- Read-path limits (Gitea #33) ------------------------------------------- +# +# These are regressions against parser properties, not against a caller. Each +# one is reproducible on an unpatched tree, and each one becomes reachable the +# moment frontmatter this instance did not write itself reaches the parser. + +# 267 bytes that compose into 672,603 nodes on traversal, parsed in 0.2 ms: +# every level references the one below it nine times, so the object graph grows +# 9**n while the input stays tiny and the parse time stays constant. A size +# limit does not touch this, which is why there is a second check. +ALIAS_BOMB = """\ +a: &a ["x", "x", "x", "x", "x", "x", "x", "x", "x"] +b: &b [*a, *a, *a, *a, *a, *a, *a, *a, *a] +c: &c [*b, *b, *b, *b, *b, *b, *b, *b, *b] +d: &d [*c, *c, *c, *c, *c, *c, *c, *c, *c] +e: &e [*d, *d, *d, *d, *d, *d, *d, *d, *d] +f: &f [*e, *e, *e, *e, *e, *e, *e, *e, *e] +""" + + +def _page(tmp_path, frontmatter_text): + path = tmp_path / "Bomb.md" + path.write_text(f"---\n{frontmatter_text}---\n\n# Bomb\n", encoding="utf-8") + return path + + +def test_an_alias_bomb_is_refused_rather_than_expanded(tmp_path): + path = _page(tmp_path, ALIAS_BOMB) + frontmatter, _, error = read_page_with_error(path) + assert frontmatter == {} + assert "alias" in error + + +def test_the_alias_check_costs_nothing_on_a_page_that_has_none(tmp_path): + """`*` is a necessary character in an alias node, so its absence proves + absence - which is the path every real page takes.""" + path = _page(tmp_path, "type: types/entity.md\ntags: [k8s, storage]\n") + frontmatter, _, error = read_page_with_error(path) + assert error is None + assert frontmatter["tags"] == ["k8s", "storage"] + + +def test_oversized_frontmatter_is_refused(tmp_path): + path = _page(tmp_path, "summary: " + ("x" * (MAX_FRONTMATTER_BYTES + 1)) + "\n") + _, _, error = read_page_with_error(path) + assert "over the" in error and "limit" in error + + +def test_a_page_at_the_size_limit_still_parses(tmp_path): + filler = "x" * (MAX_FRONTMATTER_BYTES - len("summary: \n")) + path = _page(tmp_path, f"summary: {filler}\n") + frontmatter, _, error = read_page_with_error(path) + assert error is None and frontmatter["summary"] == filler + + +def test_the_permissive_read_hands_back_the_reason_it_was_permissive(tmp_path): + path = _page(tmp_path, "type: [unclosed\n") + frontmatter, _, error = read_page_with_error(path) + assert frontmatter == {} + assert error and "invalid YAML" in error + # The historical behaviour - the reason dropped on the floor - is what let + # a broken page slip past every --field predicate looking like a miss. + assert read_page(path) == ({}, "\n# Bomb\n") + + +def test_the_strict_read_raises_instead_of_emptying(tmp_path): + path = _page(tmp_path, "type: [unclosed\n") + with pytest.raises(FrontmatterError): + read_page_strict(path) + + +def test_frontmatter_error_and_the_permissive_read_agree(tmp_path): + """One parser behind both, so they cannot describe the same file + differently - and a caller wanting both answers reads the file once.""" + for text in (ALIAS_BOMB, "type: [unclosed\n", "type: types/entity.md\n", "\n"): + path = _page(tmp_path, text) + assert frontmatter_error(path) == read_page_with_error(path)[2] + + +def test_the_fast_loader_is_used_when_libyaml_is_available(): + """The corpus parse is the largest single cost of a search and scales with + the corpus, so the factor-3 loader is not micro-tuning.""" + if hasattr(yaml, "CSafeLoader"): + assert _Loader is yaml.CSafeLoader + else: # pragma: no cover - depends on the host + assert _Loader is yaml.SafeLoader diff --git a/tools/chemenu/tests/test_instructions_cmd.py b/tools/chemenu/tests/test_instructions_cmd.py index 4c7ee02..c6144e5 100644 --- a/tools/chemenu/tests/test_instructions_cmd.py +++ b/tools/chemenu/tests/test_instructions_cmd.py @@ -11,6 +11,7 @@ import pytest import typer from chemenu import config +from chemenu.tests.conftest import use_shipped_type_specs from chemenu.commands import instructions_cmd @@ -37,6 +38,10 @@ def layer(tmp_path: Path, monkeypatch): monkeypatch.setattr(config, "INSTRUCTIONS_DIR", instructions) monkeypatch.setattr(config, "AGENTS_SKILLS_DIR", root / ".agents" / "skills") monkeypatch.setattr(config, "CLAUDE_SKILLS_DIR", root / ".claude" / "skills") + # `verify` resolves each instruction's `type: types/instruction.md`, and the + # resolver's root follows `ROOT` now - so the shipped specs have to be named + # rather than inherited. See `use_shipped_type_specs`. + use_shipped_type_specs(monkeypatch) return root diff --git a/tools/chemenu/tests/test_mcp_server.py b/tools/chemenu/tests/test_mcp_server.py new file mode 100644 index 0000000..d56a878 --- /dev/null +++ b/tools/chemenu/tests/test_mcp_server.py @@ -0,0 +1,274 @@ +"""Tests for the MCP read server (Gitea #19). + +The acceptance criteria that are not about a value coming back: + +- the wire format **is** the CLI's `--json` form, held together by a golden + test rather than by intention; +- no path of the server writes into `kb/`, `reports/` or git; +- there is no write tool, because nothing under `chemenu.commands` is + importable from it - structural, not filtered; +- every response carries the commit it was computed from; +- telemetry cannot land inside the served checkout. +""" +from __future__ import annotations + +import asyncio +import json +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +from chemenu import config + +pytest.importorskip("mcp", reason="the MCP server's dependency is optional; see " + "tools/requirements-mcp.txt") + +from chemenu.mcp.server import ( # noqa: E402 - after the skip guard + TRANSPORTS, + TraceWouldWriteIntoCorpus, + build_server, + check_trace_destination, + serve, +) + + +@pytest.fixture +def corpus(tmp_path: Path) -> Path: + """A committed instance that is not this checkout. + + Carries its own `types/`, because a served corpus is a whole instance: the + page kinds and `describe_type` are answered from the instance's own schema, + not from whichever checkout the package happens to sit in. Copied rather + than linked - a link would let a test that writes to `/types/...` + write into this repository, which is not hypothetical. + """ + root = tmp_path / "served" + entities = root / "kb" / "entities" + entities.mkdir(parents=True) + shutil.copytree(config._PACKAGE_ROOT / "types", root / "types") + (root / "kb" / "entities" / "COLLECTION.md").write_text("# entities\n", encoding="utf-8") + (entities / "Kingfisher.md").write_text( + "---\ntype: types/entity.md\nentity_type: system\nconfidence: 0.55\n" + "summary: The fixture's own system.\n---\n\n" + "# Kingfisher\n\nKingfisher is the system this fixture is about.\n", + encoding="utf-8", + ) + subprocess.run(["git", "init", "-b", "main"], cwd=root, check=True, capture_output=True) + for key, value in (("user.name", "Fixture"), ("user.email", "f@example.com")): + subprocess.run(["git", "config", key, value], cwd=root, check=True, capture_output=True) + subprocess.run(["git", "add", "-A"], cwd=root, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "corpus"], cwd=root, check=True, capture_output=True) + return root + + +def _call(server, name: str, arguments: dict | None = None) -> dict: + result = asyncio.run(server.call_tool(name, arguments or {})) + if isinstance(result, tuple): + result = result[1] + assert not result.is_error, result.content + return result.structured_content + + +def _tree(root: Path) -> dict[str, tuple[int, bytes]]: + """Every file under `root` with its size and contents, for a before/after + comparison that catches a rewrite with the same length.""" + return { + str(path.relative_to(root)): (path.stat().st_size, path.read_bytes()) + for path in sorted(root.rglob("*")) + if path.is_file() + } + + +def test_the_four_tools_are_there_and_nothing_that_writes(corpus): + server = build_server(corpus, check_trace=False) + names = {tool.name for tool in asyncio.run(server.list_tools())} + assert names == {"search", "types", "describe_type", "lint", "status"} + # Named rather than pattern-matched: the point is that these are absent + # because the functions are unreachable, and a test that only looked for + # "no tool called publish" would pass on a filtered list too. + assert names.isdisjoint({"new", "touch", "xref", "cite", "publish", "migrate", "rm"}) + + +def test_no_tool_writes_anything_into_the_corpus_or_git(corpus): + server = build_server(corpus, check_trace=False) + before = _tree(corpus) + head_before = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=corpus, capture_output=True, text=True, check=True + ).stdout + + _call(server, "search", {"query": "Kingfisher"}) + _call(server, "search", {"predicates": ["confidence<0.6"]}) + _call(server, "types") + _call(server, "describe_type", {"name": "entity"}) + _call(server, "lint") + _call(server, "status") + + assert _tree(corpus) == before + head_after = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=corpus, capture_output=True, text=True, check=True + ).stdout + assert head_after == head_before + # `lint` without `--json` writes a report into `reports/`; the server must + # only ever reach the JSON form. + assert not (corpus / "reports").exists() + assert subprocess.run( + ["git", "status", "--porcelain"], cwd=corpus, capture_output=True, text=True, check=True + ).stdout == "" + + +def test_the_wire_format_is_the_clis_json_form(corpus): + """The golden test. One contract, with the CLI as its executable + specification - so the two cannot drift while both look correct. + + The CLI is run as a subprocess against the same tree, pointed at it with + `CHEMENU_ROOT` - which is also an end-to-end check that the root resolution + works from outside the process. + """ + server = build_server(corpus, check_trace=False) + served = _call(server, "search", {"query": "Kingfisher", "limit": 5}) + + import os + + environment = {**os.environ, config.ENV_ROOT: str(corpus), "WIKI_TRACE": "0"} + completed = subprocess.run( + [str(config._PACKAGE_ROOT / "tools" / "wikitool"), + "search", "Kingfisher", "--limit", "5", "--json"], + capture_output=True, text=True, check=True, env=environment, + ) + from_cli = json.loads(completed.stdout) + + # `generated` is the CLI's date stamp and `commit`/`as_of` are the server's + # revision stamp - two answers to "when", neither of them a finding. What + # has to match is everything that describes the *corpus*. + shared = ("query", "predicates", "backend", "count", "results", "unreadable") + assert {key: served[key] for key in shared} == {key: from_cli[key] for key in shared} + + +def test_types_and_describe_match_the_cli_too(corpus): + import os + + server = build_server(corpus, check_trace=False) + environment = {**os.environ, config.ENV_ROOT: str(corpus), "WIKI_TRACE": "0"} + wikitool = str(config._PACKAGE_ROOT / "tools" / "wikitool") + + from_cli = json.loads(subprocess.run( + [wikitool, "types", "list", "--json"], + capture_output=True, text=True, check=True, env=environment, + ).stdout) + assert _call(server, "types")["types"] == from_cli + + from_cli = json.loads(subprocess.run( + [wikitool, "types", "describe", "entity", "--json"], + capture_output=True, text=True, check=True, env=environment, + ).stdout) + served = _call(server, "describe_type", {"name": "entity"}) + assert {k: v for k, v in served.items() if k not in ("commit", "as_of")} == from_cli + + +def test_every_response_carries_the_commit_it_was_computed_from(corpus): + server = build_server(corpus, check_trace=False) + head = subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=corpus, capture_output=True, text=True, check=True + ).stdout.strip() + for name in ("types", "lint", "status"): + answer = _call(server, name) + assert answer["commit"] == head + assert answer["as_of"] + + +def test_a_dirty_tree_is_stamped_with_no_commit_rather_than_a_wrong_one(corpus): + """The answer no longer corresponds to any revision, and says so instead of + naming the commit it is no longer about.""" + (corpus / "kb" / "entities" / "Latecomer.md").write_text( + "---\ntype: types/entity.md\n---\n\n# Latecomer\n", encoding="utf-8" + ) + assert _call(build_server(corpus, check_trace=False), "status")["commit"] is None + + +def test_a_bad_predicate_comes_back_as_a_tool_error_not_a_traceback(corpus): + """The SDK draws the line this depends on: a `ToolError` is a deliberate + refusal whose text reaches the caller, while any other exception is a crash + whose text stays on the server. A bad predicate is the caller's argument, so + the message that says what to write instead has to travel.""" + from mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError + + server = build_server(corpus, check_trace=False) + with pytest.raises(ToolError) as excinfo: + asyncio.run(server.call_tool("search", {"predicates": ["not a predicate"]})) + assert not isinstance(excinfo.value, UnexpectedToolError) + assert "field=value" in str(excinfo.value) + + +def test_the_server_module_cannot_reach_a_write_command(): + """Structural: import the server in a clean interpreter and nothing under + `chemenu.commands` is loaded, so there is no `publish` to expose.""" + result = subprocess.run( + [sys.executable, "-c", + "import sys, chemenu.mcp.server;" + "print([m for m in sys.modules if m.startswith('chemenu.commands')])"], + cwd=config._PACKAGE_ROOT / "tools", capture_output=True, text=True, check=True, + ) + assert result.stdout.strip() == "[]" + + +def test_tracing_into_the_served_checkout_is_refused(corpus, monkeypatch): + """The sync that keeps this checkout current is `git reset --hard`, which is + entitled to wipe `reports/`. A per-request trace written there is both lost + work and a silent way to dirty the tree the cache keys on.""" + monkeypatch.setenv("WIKI_TRACE", "1") + monkeypatch.delenv("WIKI_TRACE_DIR", raising=False) + with pytest.raises(TraceWouldWriteIntoCorpus): + check_trace_destination(corpus) + + monkeypatch.setenv("WIKI_TRACE_DIR", str(corpus / "reports" / "telemetry")) + with pytest.raises(TraceWouldWriteIntoCorpus): + check_trace_destination(corpus) + + +def test_tracing_outside_the_corpus_or_switched_off_is_accepted(corpus, tmp_path, monkeypatch): + monkeypatch.setenv("WIKI_TRACE", "0") + check_trace_destination(corpus) + + monkeypatch.setenv("WIKI_TRACE", "1") + monkeypatch.setenv("WIKI_TRACE_DIR", str(tmp_path / "traces")) + check_trace_destination(corpus) + + +def test_only_the_two_chosen_transports_are_offered(): + """`sse` is reachable through the SDK and deliberately not offered: it is + the superseded remote transport, and building on it now only moves the + migration later.""" + assert TRANSPORTS == ("stdio", "streamable-http") + with pytest.raises(ValueError, match="unknown transport"): + serve(transport="sse") + + +def test_bind_arguments_reach_only_the_transport_that_takes_them(monkeypatch): + """`run_stdio_async` accepts no host or port; handing it one is a TypeError, + not a harmless no-op. And the loopback default has to be overridable, or a + server in a container behind a reverse proxy binds an interface the proxy + cannot reach.""" + from chemenu.mcp import __main__ as entry + + seen: dict = {} + monkeypatch.setattr(entry, "serve", lambda **kwargs: seen.update(kwargs)) + + entry.main([]) + assert seen == {"transport": "stdio", "root": None} + + seen.clear() + entry.main(["--transport", "streamable-http", "--host", "0.0.0.0", "--port", "9001"]) + assert seen["transport"] == "streamable-http" + assert seen["host"] == "0.0.0.0" and seen["port"] == 9001 + + +def test_a_refused_trace_destination_stops_the_process_with_a_message(monkeypatch, capsys): + from chemenu.mcp import __main__ as entry + + monkeypatch.setenv("WIKI_TRACE", "1") + monkeypatch.delenv("WIKI_TRACE_DIR", raising=False) + assert entry.main([]) == 1 + assert "WIKI_TRACE" in capsys.readouterr().err diff --git a/tools/chemenu/tests/test_new_page.py b/tools/chemenu/tests/test_new_page.py index 6b6ce17..9f5946d 100644 --- a/tools/chemenu/tests/test_new_page.py +++ b/tools/chemenu/tests/test_new_page.py @@ -160,7 +160,10 @@ def _fixture_raw_file(monkeypatch, kb_dir, relative: str) -> None: distribution does not have.""" import chemenu.config as config + from chemenu.tests.conftest import use_shipped_type_specs + monkeypatch.setattr(config, "ROOT", kb_dir.parent) + use_shipped_type_specs(monkeypatch) path = kb_dir.parent / relative path.parent.mkdir(parents=True, exist_ok=True) path.write_text("raw fixture content\n", encoding="utf-8") diff --git a/tools/chemenu/tests/test_search.py b/tools/chemenu/tests/test_search.py index 9c051f5..4b25427 100644 --- a/tools/chemenu/tests/test_search.py +++ b/tools/chemenu/tests/test_search.py @@ -1,9 +1,18 @@ +import subprocess +import time from pathlib import Path import pytest -from chemenu.commands.search import load_pages_by_path, render_table, run_search, sort_hits -from chemenu.search import filters +from chemenu.commands.search import ( + load_pages_by_path, + render_table, + run_search, + sort_hits, + unreadable_pages, +) +from chemenu.search import filters, ripgrep +from chemenu.search.base import page_key from chemenu.search.filters import PredicateError, parse_predicate from chemenu.search.fuse import reciprocal_rank_fusion from chemenu.search.registry import UnknownBackend, resolve @@ -238,3 +247,85 @@ def test_known_fields_includes_virtual_and_real(pages): fields = filters.known_fields(pages) assert {"title", "kind", "subtype", "collection"} <= fields assert {"entity_type", "confidence", "tags"} <= fields + + +# --- Read-path limits (Gitea #33) ------------------------------------------- + + +def test_a_user_regex_never_reaches_pythons_backtracking_engine(): + """Regression for the ReDoS. `(\\w+\\s?)+$` against 114 characters of + ordinary page text does not terminate in eight seconds under `re`; the + ranking helper must not evaluate it as a pattern at all. + + Asserted by time *and* by outcome: a bound alone would pass if the branch + came back with a cheaper engine, and the outcome alone would pass while the + call still hung on a different pattern.""" + haystack = ( + "Longhorn is the distributed block storage layer for Kubernetes that " + "this cluster runs, replicated across three nodes and backed up nightly" + ) + query = SearchQuery(text=r"(\w+\s?)+$", regex=True) + started = time.perf_counter() + assert ripgrep._contains(haystack, query) is False + assert time.perf_counter() - started < 0.5 + + +def test_a_mostly_literal_regex_still_earns_its_title_boost(): + """What the deleted branch cost, and what it did not: the common case of a + pattern that happens to be plain text keeps ranking as before.""" + assert ripgrep._contains("Longhorn", SearchQuery(text="longhorn", regex=True)) is True + + +def test_ripgrep_is_called_with_a_timeout(monkeypatch, kb_dir, tmp_path): + seen = {} + + def fake_run(argv, **kwargs): + seen.update(kwargs) + return subprocess.CompletedProcess(argv, 1, "", "") + + monkeypatch.setattr(ripgrep.subprocess, "run", fake_run) + RipgrepBackend(kb_dir, tmp_path).search(SearchQuery(text="x"), {}) + assert seen["timeout"] == ripgrep.RIPGREP_TIMEOUT_SECONDS + + +def test_a_hanging_ripgrep_is_reported_as_a_failure_not_a_hang(monkeypatch, kb_dir, tmp_path): + def fake_run(argv, **kwargs): + raise subprocess.TimeoutExpired(argv, kwargs["timeout"]) + + monkeypatch.setattr(ripgrep.subprocess, "run", fake_run) + with pytest.raises(ripgrep.RipgrepFailed) as excinfo: + RipgrepBackend(kb_dir, tmp_path).search(SearchQuery(text="x"), {}) + assert "did not finish" in str(excinfo.value) + + +def test_a_page_with_broken_frontmatter_is_reported_not_lost(kb_dir, tmp_path): + """It matches no positive predicate - including the low-confidence sweep + meant to find pages in exactly that state - so silence reads as 'did not + match'. The page has to be nameable.""" + broken = kb_dir / "entities" / "Broken.md" + broken.write_text("---\ntype: [unclosed\n---\n\n# Broken\n", encoding="utf-8") + pages = load_pages_by_path(kb_dir, tmp_path) + key = page_key(broken, tmp_path) + + assert pages[key].frontmatter == {} + assert filters.apply_predicates(pages, (parse_predicate("confidence<0.6"),)) .get(key) is None + + reported = unreadable_pages(pages) + assert [entry["path"] for entry in reported] == [key] + assert "invalid YAML" in reported[0]["reason"] + + +def test_an_empty_frontmatter_block_is_not_reported_as_unreadable(kb_dir, tmp_path): + """A page may legitimately carry an empty block - there are no fields to + lose, so there is nothing the caller was not told about. `lint` still has + an opinion about it; search does not.""" + (kb_dir / "entities" / "Bare.md").write_text("---\n\n---\n\n# Bare\n", encoding="utf-8") + assert unreadable_pages(load_pages_by_path(kb_dir, tmp_path)) == [] + + +def test_a_page_with_no_frontmatter_at_all_is_reported(kb_dir, tmp_path): + """Unlike an empty block, this page has no `type:` either - it cannot match + a predicate, and nothing else would say so.""" + (kb_dir / "entities" / "Naked.md").write_text("# Naked\n\nProse only.\n", encoding="utf-8") + reported = unreadable_pages(load_pages_by_path(kb_dir, tmp_path)) + assert [entry["path"] for entry in reported] == ["kb/entities/Naked.md"] diff --git a/tools/chemenu/type_resolver.py b/tools/chemenu/type_resolver.py index 7e712da..2d4c4a3 100644 --- a/tools/chemenu/type_resolver.py +++ b/tools/chemenu/type_resolver.py @@ -26,11 +26,19 @@ class TypeResolver: """Resolves and validates type paths against type-spec files.""" def __init__(self, repo_root: Path = None): - self.repo_root = repo_root or config.ROOT + # None means "whatever the root currently resolves to" - see the + # `repo_root` property. Binding it here is what made the module-level + # `resolver` singleton answer about this checkout even when the caller + # had pointed everything else at another tree. + self._repo_root = Path(repo_root) if repo_root is not None else None self.type_cache: Dict[str, Dict[str, Any]] = {} self.schema_cache: Dict[str, Dict[str, Any]] = {} self.validator_cache: Dict[str, Any] = {} + @property + def repo_root(self) -> Path: + return self._repo_root if self._repo_root is not None else config.ROOT + def resolve_type_path(self, type_path: str, source_file: Path = None) -> Path: """Resolve a type path to an absolute, validated path. diff --git a/tools/chemenu/types_core.py b/tools/chemenu/types_core.py new file mode 100644 index 0000000..68cf6f4 --- /dev/null +++ b/tools/chemenu/types_core.py @@ -0,0 +1,90 @@ +"""The type-spec surface, with no CLI attached. + +Same split as `chemenu/search/service.py` and `chemenu/lint_core.py`: these two +functions produce the values, `commands/types_cmd.py` renders them and turns +`UnknownType` into an `ERROR` line and exit 1. + +They return exactly the structures the CLI's `--json` forms print, because that +is the wire contract the MCP server (Gitea #19) is specified against - one +contract, not two, with the CLI as its executable specification. +""" +from __future__ import annotations + +from typing import Any, Dict + +from chemenu.errors import ValidationError +from chemenu.type_resolver import resolver + + +class UnknownType(ValidationError): + """No type-spec by that name. Carries the available names, because an + unknown type is almost always a near-miss and the list is the answer.""" + + def __init__(self, name: str, available: list[str]): + self.name = name + self.available = available + super().__init__(f"No type-spec named '{name}'. Available: {', '.join(available)}") + + +def available_type_names() -> list[str]: + return sorted(fm.get("name") for _, fm in resolver.list_type_specs()) + + +def list_types() -> list[Dict[str, Any]]: + """Every type-spec under `types/`, in the shape `types list --json` prints.""" + return [ + { + "name": frontmatter.get("name"), + "type_path": type_path, + "schema": frontmatter.get("schema"), + "subtype_field": frontmatter.get("subtype_field"), + "root": frontmatter.get("root") or "kb", + "base_dir": frontmatter.get("base_dir"), + "description": frontmatter.get("description"), + } + for type_path, frontmatter in resolver.list_type_specs() + ] + + +def describe_type(name: str) -> Dict[str, Any]: + """One type's full contract, in the shape `types describe --json` prints. + + Raises `UnknownType` rather than exiting: the caller may be a server, for + which an exit code is not an answer. + """ + type_path = resolver.find_type_by_name(name) + if type_path is None: + raise UnknownType(name, available_type_names()) + + type_spec = resolver.load_type_spec(type_path) + frontmatter = type_spec["frontmatter"] + schema = resolver.get_schema(type_path) + + fields: list[Dict[str, Any]] = [] + if schema is not None: + required = set(schema.get("required", [])) + for field_name, field_schema in schema.get("properties", {}).items(): + fields.append({ + "field": field_name, + "required": field_name in required, + "type": field_schema.get("type"), + "enum": field_schema.get("enum"), + }) + + return { + "name": frontmatter.get("name"), + "type_path": type_path, + "description": frontmatter.get("description"), + "schema": frontmatter.get("schema"), + "subtype_field": frontmatter.get("subtype_field"), + # `root` was previously read straight from the frontmatter by the + # renderer and left out of the JSON payload, so `describe --json` could + # not tell you where a type's pages go while `list --json` could - + # `types/instruction.md` declares `root: repo`. Carried here, in the + # same shape `list` uses. + "root": frontmatter.get("root") or "kb", + "base_dir": frontmatter.get("base_dir"), + "title_prefix": frontmatter.get("title_prefix"), + "fields": fields, + "body": type_spec["body"].strip(), + } diff --git a/tools/requirements-mcp.txt b/tools/requirements-mcp.txt new file mode 100644 index 0000000..0ec875d --- /dev/null +++ b/tools/requirements-mcp.txt @@ -0,0 +1,13 @@ +# The MCP read server's dependency, deliberately not in requirements.txt. +# +# `requirements.txt` describes what an *instance* needs to run `wikitool`, and +# it is what `dist export` ships. The server is an optional second consumer: +# an instance that only ever uses the CLI should not be made to install +# pydantic, starlette, uvicorn and cryptography to do it. +# +# tools/.venv/bin/pip install -r tools/requirements-mcp.txt +# WIKI_TRACE=0 tools/.venv/bin/python -m chemenu.mcp --transport stdio +# +# The tests under `chemenu/tests/test_mcp_server.py` skip without it; CI +# installs it, so they do run. +mcp>=2.0