feat(registry): make the repository scan clean for the HOL Plugin Registry - #454

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

ZETETIC-REVIEW: REQUEST_CHANGES

Résumé

La PR fait exactement ce qu'elle annonce pour le scanner HOL (workflow SHA-pinné vérifié, manifeste Codex complété, LICENSE/SECURITY dupliqués et identiques, package-lock.json cohérent, mot de passe de test remplacé par un secret généré). Mais le gate craftsmanship du dépôt (scripts/check_craftsmanship.py, obligatoire dans CI Green via ci-green.needs: [..., craftsmanship, ...]) échoue réellement sur le commit de tête 2c7843c1 : le changement fait passer lowpriv_role_url (dans tests_py/invariants/test_pg_schema_provision_live.py) de 39 à 42 lignes, au-dessus de la limite locale de 40 lignes/méthode (scripts/craftsmanship_rules.py::METHOD_LINE_LIMIT), et cette violation est absente de la baseline base-ref (.craftsmanship-baseline.json à origin/main comme dans l'arbre courant : 3 entrées, aucune sur ce fichier). C'est une régression neuve introduite par ce diff, pas de la dette préexistante. Le plan de test de la PR affirme "check_craftsmanship.py → OK", ce qui est faux sur ce commit exact (vérifié en le rejouant moi-même, aucune ambiguïté d'environnement : script Python pur, sans dépendance PostgreSQL).

Move 0 — Réconciliation du ledger et vérification anti-esquive (§13.2 + §14)

• Réconciliation du ledger : le corps de la PR fournit un "Test plan" équivalent à un ledger (commandes + résultats attendus) ; tous les chemins du diff sont couverts par au moins une commande de vérification listée. Pas de branche/mode dégradé non cartographié détecté.
• Rationalisations "seen-defect" : aucune trouvée. La phrase sur trust_helpers.check_percent scorant un check à max_points==0 comme 0% au lieu de l'exclure du dénominateur est une limite du scanner tiers, énoncée platement, hors du diff — pas une esquive d'un défaut que la PR aurait elle-même introduit et refuserait de corriger.
• Court-circuit : N/A pour ce mécanisme — mais voir plus bas : le plan de test affirme un résultat ("check_craftsmanship.py → OK") qui s'avère faux à la vérification indépendante. Ce n'est pas une rationalisation verbale d'un défaut connu (aucune langue du type "pre-existing/unrelated/out of scope" n'est employée), donc ce n'est pas un REFUSED au sens strict du §14 — c'est une affirmation de vérification incorrecte, traitée comme un blocage ordinaire (Move 5 / craftsmanship gate) plutôt qu'un refus immédiat.

Calibration des enjeux (Move 7) — classification objective

• Classification : High
• Critère : les deux fichiers de code touchés par du contenu logique (scripts/check_version_surfaces.py, tests_py/invariants/test_pg_schema_provision_live.py) ont chacun 2 auteurs distincts sur les 90 derniers jours (git log --format='%an' --since='90 days ago' -- <file> | sort -u | wc -l = 2 pour les deux) — critère objectif de Move 7. Le workflow CI touche aussi la surface sécurité/supply-chain (permissions, SARIF).
• Profondeur appliquée : Moves 1-6 complets.

Préférences du lead (CAP-2)

• /memories/reviewer-prefs/ : aucune préférence de lead trouvée pour ce dépôt — N/A.

Vérification du contenu (par point de la demande)

1. Plugin Codex (plugins/hypermnesia-mcp-codex/)

• Manifest JSON valide (python3 -m json.tool OK) ; tous les chemins référencés (assets/icon.svg, assets/screenshot-overview.png, assets/screenshot-benchmarks.png) existent bien dans l'arbre.
• tests_py/scripts/test_codex_plugin_contract.py : toujours vert (32/32 avec test_version_surfaces.py), et l'assertion for unsupported in ("hooks", "skills", "apps", "agents", "postInstall"): assert unsupported not in plugin reste satisfaite — les nouvelles clés ajoutées (privacyPolicyURL, termsOfServiceURL, composerIcon, logo, screenshots) sont toutes sous interface, pas au niveau racine du manifeste.
• README : la liste des dix outils "lean" (query_methodology, remember, recall, unified_search, recall_hierarchical, consolidate, memory_stats, check_setup, wiki_read, wiki_list) correspond exactement, élément par élément, à mcp_server/tool_profiles.py::LEAN_TOOL_NAMES.
• LICENSE : diff LICENSE plugins/hypermnesia-mcp-codex/LICENSE (et les deux shims dépréciés) → identiques octet pour octet.
• icon.svg : 497 octets, pas de <script> ni de gestionnaire d'événement — pas de vecteur XSS/exécution.
• Verdict sur ce point : conforme, aucun écart avec la revendication.

2. package-lock.json + check_version_surfaces.py

• package.json, package-lock.json (deux occurrences de version) et pyproject.toml sont tous à 4.18.0 — cohérents.
• Arithmétique du commentaire (14→16 sites, 11→12 fichiers) vérifiée par le test test_sixteen_surfaces_are_registered et confirmée à l'exécution : python3 scripts/check_version_surfaces.py → 16 site(s) checked, all agree.
• Étape manuelle supplémentaire au bump de version ? Non : check_version_surfaces.py tourne déjà dans ci.yml (lignes 482 et 984) et fait partie de la même procédure de vérification déjà existante ; l'ajout de package-lock.json comme 15e/16e surface ne change rien au processus de release documenté dans CLAUDE.md § Releasing — un futur bump désynchronisé échouera en CI comme avant, sans étape manuelle nouvelle.

3. test_pg_schema_provision_live.py — mot de passe généré

• secrets.token_urlsafe(16) produit un alphabet [A-Za-z0-9_-] (base64 URL-safe sans padding) — aucun caractère ne peut casser le littéral SQL PASSWORD '...' (pas de guillemet simple, pas de backslash) ni l'URL construite via urlunsplit (- et _ sont des caractères "unreserved" RFC 3986, aucun échappement requis dans le userinfo). Vérifié par lecture directe du code, pas par supposition.
• Mais ce changement (+3 lignes nettes : import + commentaire de 3 lignes remplaçant une ligne) fait passer la fonction lowpriv_role_url de 39 à 42 lignes, au-dessus de la limite locale de 40 (CLAUDE.md § Code Style, appliquée par scripts/check_craftsmanship.py, gate obligatoire dans CI Green). Voir Move 5 ci-dessous — Blocking.

4. .github/workflows/hol-plugin-scanner.yml

• actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 : confirmé via gh api repos/actions/checkout/git/refs/tags/v7.0.1 → même SHA, commit signé et vérifié GitHub ; identique au pin utilisé par ci.yml (grep confirmé, 5 occurrences).
• hashgraph-online/ai-plugin-scanner-action@5943e732726b177c9a46baa57f6df5b493ca3429 : confirmé via gh api repos/hashgraph-online/ai-plugin-scanner-action/git/refs/tags/v1.2.576 → même SHA.
• Permissions : contents: read au niveau workflow, contents: read + security-events: write au niveau job — le minimum nécessaire pour l'upload SARIF, rien de plus (pas de pull-requests: write, pas de id-token). Conforme à la règle "pinned-dependencies"/least-privilege d'OpenSSF Scorecard (les deux actions sont épinglées par SHA complet, avec commentaire de tag lisible).
• scripts/check_ci_gate_complete.py : lu directement — CI_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "ci.yml" (constante en dur) : le script ne gouverne que ci.yml, jamais les autres workflows. La nouvelle hol-plugin-scanner.yml est donc légitimement hors de son périmètre, cohérent avec l'affirmation du corps de la PR ("Deliberately outside the CI Green gate"). Exécution locale : python3 scripts/check_ci_gate_complete.py → CI gate completeness: OK.
• État CI réel (gh pr checks 454) : HOL plugin scan → pass (58s). Le job Craftsmanship Gate était encore pending au moment de la requête — mais reproduit localement en échec déterministe sur le commit exact de tête (voir Move 5).

5. CHANGELOG.md / docs/codex-plugin.md

• Le CHANGELOG ([Unreleased] → Added/Fixed) correspond fidèlement au diff : workflow scanner, complétion du paquet Codex, package-lock.json, remplacement du mot de passe littéral. Aucune sur-affirmation détectée.
• docs/codex-plugin.md : contenu factuellement correct, mais defaut de forme non bloquant — la phrase ajoutée ("...duplicated there on purpose.") est collée sans saut de ligne à la phrase préexistante ("This preserves Cortex's Claude contract:"), ce qui rend le paragraphe difficile à lire dans le rendu Markdown source (n'affecte pas le rendu HTML final, juste la lisibilité du diff/source). Non-blocking.

Layer check (Move 1)

SOLID audit (Move 2)

Aucune classe/fonction métier modifiée. lowpriv_role_url reste une seule responsabilité (créer/nettoyer un rôle PG à faible privilège) — pas de violation SRP/OCP/LSP/ISP/DIP, seulement une question de taille (Move 5).

Wiring & contract drift (Move 3)

• Nouveaux symboles publics : 2 entrées SURFACES (données, pas du code appelant) — toutes deux couvertes par de nouveaux tests dédiés (test_package_lock_root_version_desync_is_reported, test_package_lock_root_package_entry_desync_is_reported).
• Pas de changement de signature de fonction existante affectant des appelants externes.
• Pas de code mort, pas de TODO sans ticket, pas de print/console.log de debug détecté dans le diff.

Test adequacy (Move 4)

• Nouvelles branches (désynchronisation package-lock.json racine vs packages[""].version) : chacune a un test dédié qui vérifie le message d'erreur précis (postcondition), pas seulement l'échec générique. Bon niveau de rigueur mutationnelle informelle : inverser l'ordre des deux assertions assertIn/assertNotIn romprait le test.
• Le remplacement du mot de passe littéral n'a pas de test dédié à sa "sécurité" (alphabet safe dans le littéral SQL et l'URL) — c'est acceptable ici : la fonction est un fixture de test PostgreSQL, déjà exercée existentiellement par la suite invariants quand PostgreSQL est disponible ; ajouter un test unitaire sur l'alphabet de secrets.token_urlsafe reviendrait à tester la stdlib, pas notre code. Non-blocking.
• Pas de caractérisation nécessaire : comportement fonctionnel inchangé (seule la valeur du mot de passe change, testée transitivement).

Complexity & structure (Move 5)

• Blocking : tests_py/invariants/test_pg_schema_provision_live.py::lowpriv_role_url passe de 39 à 42 lignes à cause de ce diff (+1 import, -1/+4 lignes sur le commentaire+password), dépassant la limite locale de 40 lignes/méthode (CLAUDE.md § Code Style, scripts/craftsmanship_rules.py::METHOD_LINE_LIMIT = 40). Confirmé par exécution : python3 scripts/check_craftsmanship.py → Craftsmanship gate: NEW violations (not in the base-ref baseline): [method-size] tests_py/invariants/test_pg_schema_provision_live.py: lowpriv_role_url, exit code 1. La baseline base-ref (git show origin/main:.craftsmanship-baseline.json, 3 entrées) ne contient pas cette fonction — ce n'est pas de la dette préexistante, c'est une régression neuve introduite par ce diff. Ce gate est obligatoire dans CI Green (ci-green.needs inclut craftsmanship, ligne 826 de ci.yml).
• Correctif minimal : raccourcir le commentaire ajouté (une ligne au lieu de trois suffit à documenter le "pourquoi") pour repasser sous 40 lignes, ou extraire la création/nettoyage du rôle dans un petit helper partagé si le commentaire complet doit être conservé.
• Aucun autre red flag de taille détecté ailleurs dans le diff.
• Sur-ingénierie (Move 5, non bloquant) : aucune détectée.

Security & hygiene (Move 6)

• Sécurité : aucun secret committé ; le seul "high finding" que la PR corrige (mot de passe littéral) est effectivement retiré et remplacé par une valeur générée à l'alphabet sûr. Les deux SHA de GitHub Actions sont vérifiés indépendamment auprès de l'upstream (voir §4). Permissions du nouveau workflow : minimales (contents: read + security-events: write uniquement pour l'upload SARIF).
• Hygiène des commits : un seul commit feat(registry): make the repository scan clean for the HOL Plugin Registry — format conventionnel respecté. Le commit regroupe plusieurs sous-changements (workflow + manifeste + version surfaces + fix de test + shims dépréciés), mais tous servent un seul objectif cohérent et documenté (faire passer le score du scanner HOL) — pas de scope creep au sens de mélange fonctionnalité/refactor/bump de dépendance sans rapport. Non-blocking.

Gates exécutés (transcriptions brutes)

$ ruff check .
All checks passed!

$ ruff format --check .
1245 files already formatted

$ .venv/bin/python -m pytest tests_py/scripts/test_version_surfaces.py tests_py/scripts/test_codex_plugin_contract.py -q
................................                                         [100%]
32 passed, 1 warning in 0.50s

$ python3 scripts/check_version_surfaces.py
version surfaces OK (16 site(s) checked, all agree with pyproject.toml)

$ python3 scripts/check_doc_claims.py
doc claims OK (1 declared not-a-claim exemption(s))
  CONTRIBUTING.md:36: exempt from the tests claim

$ python3 scripts/check_ci_gate_complete.py
CI gate completeness: OK

$ python3 scripts/check_craftsmanship.py
Craftsmanship gate: NEW violations (not in the base-ref baseline):
  - [method-size] tests_py/invariants/test_pg_schema_provision_live.py: lowpriv_role_url
(exit code 1)

État CI (gh pr checks 454, interrogé une seule fois, sans attente) :

• HOL plugin scan : pass (58s)
• Lint, Test (Windows, SQLite backend), Release dependency set, Docker Build (devcontainer image), Fuzz (PR batch) ×2, Analyze ×3, CodeQL, agree with the upstream identity contract, plugin-scanner : pass
• Craftsmanship Gate, Type Check, Build Package, Docker Build (runtime image), Test (SQLite backend), Docker Smoke, Test (Python 3.10/3.11/3.12/3.13), Validate MCP host configurations : pending au moment de la requête
• Je n'ai pas attendu ces jobs (gh run watch non utilisé) ; la reproduction locale de check_craftsmanship.py sur le SHA exact de tête suffit à établir que Craftsmanship Gate échouera lorsqu'il se termine.

Issues

Blocking

• [tests_py/invariants/test_pg_schema_provision_live.py — fonction lowpriv_role_url, ~lignes 84-125] Violation de taille de méthode (CLAUDE.md § Code Style, 40 lignes max, appliquée par scripts/check_craftsmanship.py, gate obligatoire dans CI Green) — le diff fait passer la fonction de 39 à 42 lignes. Correctif : raccourcir le commentaire ajouté à une ligne, ou extraire un helper _create_lowpriv_role(conn, role, password) / _drop_lowpriv_role(...) pour repasser sous la limite. Confirmé par exécution reproductible sur le commit de tête exact 2c7843c1, exit code 1.

Non-blocking

• [docs/codex-plugin.md] La phrase ajoutée est concaténée sans saut de ligne à la phrase suivante préexistante — lisibilité de la source Markdown seulement, aucun effet sur le rendu ou la sémantique.
• [corps de la PR, section "Test plan"] Affirme check_craftsmanship.py → OK, ce qui est faux sur le commit de tête actuel (voir Blocking). Vraisemblablement une exécution antérieure à un commit ultérieur (le corps mentionne un rebase sur 54084e59, donc soit le test a été rejoué avant le dernier ajustement du commentaire, soit l'environnement d'exécution de l'auteur diffère). À corriger dans le prochain commit avec une ré-exécution effective avant de republier le plan de test.

Hand-offs

• Aucun renvoi nécessaire vers un autre agent — le correctif est mécanique (raccourcir/refactorer une méthode de test de 2-3 lignes) et relève directement de refactorer ou de l'auteur lui-même, pas d'un des blind spots listés (pas de root-cause distinct, pas de question de décomposition structurelle, pas de correction formelle, pas de menace de sécurité nécessitant un threat model, pas de revendication de performance).

Enregistrements mémoire

• Aucun enregistrement remember distinct effectué : cette révision est ponctuelle et le fait marquant (craftsmanship gate cassé par un diff en apparence anodin sur un simple remplacement de mot de passe littéral) est suffisamment capturé dans ce commentaire PR lui-même, qui sert de source vérifiable (lien PR feat(registry): make the repository scan clean for the HOL Plugin Registry #454) si une leçon générale devait être proposée à l'orchestrateur plus tard.

Verdict

REQUEST_CHANGES

Changement minimal pour débloquer le merge : ramener lowpriv_role_url sous la limite de 40 lignes (raccourcir le commentaire ajouté à une ligne suffit très probablement) et republier le plan de test avec une ré-exécution effective de python3 scripts/check_craftsmanship.py montrant exit code 0. Tout le reste de la PR est vérifié conforme à ce qu'elle revendique (pins SHA exacts, manifeste valide, arithmétique des surfaces de version correcte, alphabet du secret sûr dans le littéral SQL et l'URL, périmètre de check_ci_gate_complete.py confirmé limité à ci.yml).

ZETETIC-REVIEW: APPROVE

Ce commentaire remplace mon commentaire round 1 (#454 (comment)), qui concluait REQUEST_CHANGES sur un seul finding bloquant.

Ce qui a changé (round 2)

Commit 178527af (« fix(tests): keep lowpriv_role_url under the 40-line method cap ») ne touche que tests_py/invariants/test_pg_schema_provision_live.py : le commentaire de 3 lignes expliquant le choix de secrets.token_urlsafe(16) devient un commentaire de fin de ligne sur password = secrets.token_urlsafe(16) # per-run; a literal scans as a secret. Vérifié par git show --stat 178527af : 1 fichier, +1/-4 lignes, aucun changement de logique. git diff --stat 54084e59...178527af confirme que tout le reste du diff cumulatif est identique à ce que j'avais déjà vérifié au round 1.

Re-vérification indépendante sur le commit de tête exact 178527af

• git rev-parse HEAD local = 178527af799d1e700b7a003e2f55f5b81d5c42a7 = headRefOid rapporté par gh pr view 454 — je vérifie bien le bon commit.
• Taille de la fonction mesurée par AST (même méthode qu'au round 1, pas une estimation) : lowpriv_role_url fait maintenant 39 lignes (lignes 84-122), sous la limite locale de 40 (CLAUDE.md § Code Style, scripts/craftsmanship_rules.py::METHOD_LINE_LIMIT).
• Le commentaire déplacé documente toujours le "pourquoi" (littéral = faux positif de scanner de secrets), donc aucune perte d'information de revue.
• La logique du littéral SQL PASSWORD '{password}' et de l'URL construite via urlunsplit est inchangée — mes vérifications round 1 sur la sécurité de l'alphabet de secrets.token_urlsafe (A-Za-z0-9_-, aucun caractère à échapper dans un littéral SQL ni dans un userinfo d'URL RFC 3986) restent valables telles quelles.

Gates rejoués sur 178527af, transcriptions brutes

$ python3 scripts/check_craftsmanship.py
Craftsmanship gate: OK
(exit 0)

$ ruff check .
All checks passed!
(exit 0)

$ ruff check tests_py/invariants/test_pg_schema_provision_live.py
All checks passed!
(exit 0)

$ ruff format --check .
1245 files already formatted
(exit 0)

$ .venv/bin/python -m pytest tests_py/scripts/test_version_surfaces.py tests_py/scripts/test_codex_plugin_contract.py -q
................................                                         [100%]
32 passed, 1 warning in 0.47s

$ python3 scripts/check_version_surfaces.py
version surfaces OK (16 site(s) checked, all agree with pyproject.toml)

$ python3 scripts/check_ci_gate_complete.py
CI gate completeness: OK

$ python3 scripts/check_doc_claims.py
doc claims OK (1 declared not-a-claim exemption(s))
  CONTRIBUTING.md:36: exempt from the tests claim

Le seul finding bloquant du round 1 est donc corrigé et vérifié corrigé, pas seulement affirmé corrigé.

État CI (gh pr checks 454, interrogé une seule fois, sans attente ni gh run watch)

Requête faite juste après le push de 178527af — la plupart des jobs sont encore pending (build en cours) :

• HOL plugin scan : pass (56s)
• plugin-scanner, agree with the upstream identity contract : pass
• Analyze (×3), Fuzz (PR batch) (×2), Fuzz (scheduled batch) : pending/skipping (build pas terminé au moment de la requête)
• Le job Craftsmanship Gate (celui qui avait échoué au round 1) n'apparaissait pas encore dans cette liste au moment de la requête (pipeline encore en démarrage) — mais je l'ai reproduit localement, déterministe et sans dépendance externe (script Python pur, python3 scripts/check_craftsmanship.py → OK, exit 0), sur le SHA exact de la tête de PR. Je n'ai pas attendu la fin du pipeline, conformément à la consigne.

Findings restants

Aucun nouveau finding bloquant. Les deux points non-bloquants relevés au round 1 restent d'actualité mais ne changent pas le verdict :

• [docs/codex-plugin.md] Phrase ajoutée concaténée sans saut de ligne à la phrase suivante préexistante — lisibilité de la source Markdown uniquement, aucun effet sur le rendu.
• [historique] Le plan de test du round 1 affirmait à tort que check_craftsmanship.py passait ; ce n'est plus pertinent puisque round 2 le fait effectivement passer et je l'ai revérifié moi-même plutôt que de faire confiance à l'affirmation du coordinateur.

Vérification adversariale avant APPROVE (obligatoire, Move 12/CR-4)

1. Faux positifs résiduels / sur-correction : aucun — le fix est strictement ciblé sur la violation signalée, sans effet de bord (diff = 1 fichier, +1/-4 lignes, logique inchangée).
2. Cas manqués : git diff --stat 54084e59...178527af confirme qu'aucun autre fichier du diff cumulatif n'a été modifié depuis le round 1 — pas de régression introduite ailleurs.
3. Robustesse : le comportement d'exécution (rôle PG, mot de passe, URL) est bit-à-bit identique ; seul un commentaire a changé de position.
4. Adéquation des tests : sans objet pour un changement de commentaire pur — aucune assertion de test ne porte sur le nombre de lignes d'un commentaire ; les 32 tests des deux modules ciblés restent verts.

Aucun finding bloquant confirmé, les quatre angles ont pu être examinés → APPROVE.

Verdict

APPROVE

Le finding bloquant du round 1 (dépassement de la limite locale de 40 lignes/méthode sur lowpriv_role_url, gate craftsmanship obligatoire dans CI Green) est corrigé et revérifié indépendamment sur le commit de tête exact 178527af. Tous les gates rejoués localement passent. Le reste de la PR reste conforme à ce qui avait déjà été vérifié au round 1 (pins SHA exacts pour actions/checkout et hashgraph-online/ai-plugin-scanner-action, manifeste Codex valide avec tous les assets référencés présents, LICENSE dupliquées identiques, arithmétique des surfaces de version correcte, alphabet du secret généré sûr dans le littéral SQL et l'URL, périmètre de check_ci_gate_complete.py limité à ci.yml).