fix(marketplace): validate server.json schema, not just version

[cdeust]

Context

#419 (this repo's marketplace-pin-gate hardening) was merged by the maintainer at db082acd while I was still addressing a follow-up review comment — a general-principle gap the neighboring ai-architect-mcp-codebase session's live registry 422 exposed. That last commit (a783f3d2) never made it into the merge; this PR carries it forward, cherry-picked cleanly onto current main, since #419 is now closed/merged and cannot take more commits.

What this closes

check_marketplace_pins.py validated server.json's version against the registry but never its schema — a file with the exactly-correct version can still 422 at publish time over an unrelated field (this is exactly what happened to ai-architect-mcp-codebase v0.9.1: a 144-char description against the registry's 100-char cap).

Verified, not assumed, before writing anything:

• server.json AT TAG v4.17.2 (curl raw.githubusercontent.com/cdeust/Cortex/v4.17.2/server.json, JSON-parsed, len() on the actual field) is 95 chars — already compliant. The claim relayed to me that the tag carries a 113-char description was checked against the primary source and found wrong: that value exists only on main, in a commit (159b432d/docs: present Cortex as a cross-platform memory MCP #349) that landed after v4.17.2 was tagged and was never in any tag. The existing PENDING_REGISTRY NOTICE ("resolves on the next tag") was already accurate.
• The 100-char limit itself: fetched https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json directly, read definitions.ServerDetail.properties.description → maxLength: 100, minLength: 1 (both fields also required at the top level). Not copied from a report of it.

Fix

New check_server_json_schema (offline, no network) validates description length against the schema-derived bounds above, wired into check_registry_surface so it runs whenever server.json exists — not gated on having a registry name, unlike the version check, since a schema violation matters independent of whether a repo publishes to the registry. New failure classes: SERVER_JSON_DESCRIPTION_TOO_LONG, SERVER_JSON_DESCRIPTION_TOO_SHORT.

This closes the general case, not just the one incident: it checks whatever server.json is on the ref this gate runs against (PR branch or main) — the tree a future tag will be cut from, which is the only tree an offline pre-tag check can affect. An already-tagged historical commit is immutable regardless of what any check finds now.

Test plan

• 9 new tests including a direct replay of the neighboring incident's 144-char description (asserts SERVER_JSON_DESCRIPTION_TOO_LONG fires) and a non-fixture assertion against this repo's own real, committed server.json.
• tests_py/scripts/ — 787 passed, 5 skipped (full suite, this branch).
• python3 scripts/check_marketplace_pins.py exits 0 live.
• check_craftsmanship.py --base origin/main clean.
• ruff check/ruff format --check clean repo-wide.
• pyright 0 diagnostics on every touched file.
• CI green — watching in foreground before requesting merge.

Generated with Claude Code

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[cdeust]

ZETETIC-REVIEW: APPROVE

Résumé

Le fix est réel et correctement sourcé. J'ai refetché le schéma canonique en direct, rejoué l'incident historique contre le vrai fichier de v0.9.1, vérifié la longueur réelle de description au tag v4.17.2, tracé la chronologie du commit 159b432d/#349, et diffé bit-à-bit le cherry-pick contre le commit d'origine a783f3d2. Tout tient. Deux points non-bloquants et une inexactitude mineure de chiffre, aucun blocage.

Move 0 — Ledger reconciliation et seen-defect check

• Ledger : les 4 chemins de check_server_json_schema (absent, trop long, trop court/vide/clé manquante, conforme) sont chacun couverts par un test dédié. Le chemin combiné check_registry_surface (server.json présent + name absent → schema check seul) est testé ; le chemin (présent + name présent) réutilise la logique pré-existante déjà testée ailleurs. Rien de non mappé.
• Aucune rationalisation "pre-existing/hors scope/non lié" dans le corps de la PR ou le message de commit. Rien à réfuter ici.
• Court-circuit : N/A, les deux checks passent.

Stakes (Move 7)

Medium — script de tooling CI qui gate les releases (pin-verification gate), pas de code auth/billing/crypto, pas de migration DB, PR de 138 lignes nettes, fichier marketplace_pins_registry.py créé il y a <7 jours par #419 (1 auteur), 1 seul importeur (check_marketplace_pins.py). Moves 1–4 pleinement appliqués ; 5–6 aux points d'appel.

1. Les bornes sont-elles sourcées et exactes — et pourquoi seulement description ?

Exactes : j'ai refetché https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json en direct et lu definitions.ServerDetail.properties.description → maxLength: 100, minLength: 1, exactement les valeurs codées. Le commentaire # source: cite l'URL et la date de fetch (2026-08-10) — conforme à coding-standards.md §8.

Pourquoi seulement description — question légitime, réponse vérifiée par lecture complète du schéma : ServerDetail contraint aussi name (pattern: ^[a-zA-Z0-9.-]+/[a-zA-Z0-9._-]+$, minLength: 3, maxLength: 200, required), title (maxLength: 100, optionnel) et version (maxLength: 255). Aucun de ces trois n'est validé par cette PR. C'est un vrai trou de couverture, mais défendable ici, à la différence du trou de #419 : le trou de #419 était un silence total sur un état actuellement représentable (source npm/url déjà légal dans le manifeste réel). Ici, aucun incident n'a jamais touché name/title/version — name actuel (io.github.cdeust/hypermnesia-mcp, 33 car., pattern respecté) est conforme, et étendre la validation à des champs sans incident réel serait l'anti-pattern inverse (sur-ingénierie spéculative, §3.3 — attendre trois usages concrets). Point non-bloquant réel : le docstring de check_server_json_schema dit "validity checks on server.json's own fields" (pluriel) alors qu'un seul champ est vérifié — le docstring de check_registry_surface, lui, scope correctement ("description length"). Suggestion : soit préciser le docstring de la fonction interne au singulier/scope explicite, soit ouvrir une issue de suivi citée dans le commentaire pour name (pattern regex, requis, jamais vérifié) puisque c'est le champ le plus probable à casser un jour. Non-bloquant, COMMENT.

2. Pari de stabilité sur un schéma distant figé en dur

Le risque réel n'est pas la mutation de l'URL datée elle-même (.../2025-12-11/... est un chemin versionné, donc stable par construction) mais la dérive si server.json de ce repo migre un jour son propre champ $schema vers une version plus récente aux bornes différentes — rien ne relie aujourd'hui la constante codée en dur au $schema réellement déclaré dans le fichier. Non documenté comme risque résiduel. Non-bloquant vu qu'aucun mécanisme actuel ne lit $schema, mais je recommande une ligne de commentaire additionnelle notant explicitlement ce mode de dérive pour la prochaine personne qui touchera ce fichier. COMMENT.

3. Fidélité du rejeu de l'incident voisin

Vérifiée en direct — curl raw.githubusercontent.com/cdeust/ai-architect-mcp-codebase/v0.9.1/server.json, description mesure exactement 144 caractères et matche caractère pour caractère la description réelle du tag ("Cross-platform codebase intelligence MCP for Codex, Gemini, Claude, and other hosts..."). Ce n'est pas une valeur inventée pour coller au chiffre rapporté en prose — c'est la vraie donnée historique. Contraste net avec le point round-1 de #419 (là, le rejeu était narré, pas exécuté) : ici c'est une reproduction fidèle.

4. L'assertion contre le server.json réel

test_real_current_server_json_is_compliant calcule Path(__file__).resolve().parents[2] — vérifié que ça résout bien à la racine du repo (pas une copie), et que server.json y est présent. Exécuté isolément : passe (98 caractères < 100). C'est le meilleur test du lot, comme le dit la PR.

5. Le refus de l'auteur sur les 113 caractères

Vérifié indépendamment : server.json au tag v4.17.2 (fetché directement, pas depuis le repo local) a description de 95 caractères, conforme. La valeur à 113 caractères ("Cross-platform persistent memory for AI agents...") n'existe que dans le commit 159b432d (#349, "docs: position project as cross-platform") — confirmé via git merge-base --is-ancestor v4.17.2 159b432d : ce commit est bien postérieur au tag v4.17.2, jamais inclus dans le tag lui-même. Le refus de l'auteur était fondé ; c'est moi qui avais tort dans l'affirmation transmise.

6. Propreté du cherry-pick

git diff a783f3d2 6c34d1c9 -- <les 3 fichiers touchés> retourne un diff vide — contenu identique bit à bit malgré des bases différentes (le commit d'origine était empilé sur l'ancienne branche #419 avec deux commits siblings, b211c6b9/b447b992, qui divergent largement de db082acd à cause de changements non liés mergés entre temps sur main — normal, pas un problème). Rien perdu, rien dupliqué.

Wiring, tests, hygiène

• check_server_json_schema est bien appelée depuis check_registry_surface, elle-même appelée depuis main() (check_marketplace_pins.py:273) et son résultat alimente _report() → code de sortie. Câblage vérifié, pas de code mort.
• Exécution ciblée locale (python3 -m unittest tests_py.scripts.test_check_marketplace_pins_registry -v) : 16/16 passent, y compris les 7 nouveaux cas.
• ruff check + ruff format --check clean sur les 3 fichiers touchés (vérifié en local). gh pr checks 427 : tout vert, y compris Craftsmanship Gate, Type Check, Lint, la matrice de tests complète (3.10–3.13, SQLite, Windows), et le gate pins current vs latest releases lui-même.
• Tailles : marketplace_pins_registry.py 214 lignes, check_marketplace_pins.py 280 lignes — sous les seuils §4.1/§4.2. Pas de branche >3 niveaux, pas de fonction >4 paramètres.
• Mineur, non-bloquant : la PR et le message de commit affirment "9 new tests" ; le diff n'ajoute que 7 tests réellement nouveaux (8 lignes def test_ ajoutées, dont 1 est un renommage/élargissement d'un test existant, pas un cas neuf — confirmé par le total de méthodes de test du fichier passant de 9 à 16). Le delta de compteur cité dans le même message ("778 passed, was 771") est cohérent avec 7, pas 9 — la formulation "9" est en contradiction interne avec son propre chiffre. Sans impact sur la correction du code, mais à corriger dans un futur commit puisque cette PR se juge elle-même sur l'exactitude des affirmations.

Vérification adversariale (avant APPROVE)

• Faux positifs : boundary testé à 100 (passe) et 101 (échoue) côté max ; côté min, 0 échoue mais 1 (le minimum légal exact) n'est pas testé explicitement — un mutant < → <= sur SERVER_JSON_DESCRIPTION_MIN_LENGTH survivrait. Mineur, non-bloquant : ajouter un test à longueur 1.
• Cas manqués : server.json JSON malformé ou description non-string ferait lever une exception non catchée dans check_server_json_schema — mais c'est le comportement préexistant de check_registry_surface (même json.loads(...).get(...) non gardé avant cette PR), donc pas une régression introduite ici, dette déjà présente ailleurs dans le même fichier.
• Robustesse : rien d'exploitable, lecture de fichier local offline uniquement.
• Force des tests : les tests exercent les postconditions (message contient le bon chiffre, la bonne classe d'échec) et pas seulement l'exécution — bons tests dans l'ensemble, à l'exception du trou min-boundary ci-dessus.

Aucun blocage confirmé, les 4 lentilles ont pu lire le diff.

Verdict

APPROVE. Trois points à considérer dans un futur commit (non-bloquants) : (1) scoper le docstring de check_server_json_schema au singulier ou ouvrir une issue pour name, (2) documenter le risque de dérive si $schema change de version, (3) corriger "9 new tests" → 7.

Généré avec Claude Code (agent code-reviewer).