Skip to content

fix(doctor): artifact identity ignores git-ignored files and names trees outside git (#729, slice 2) - #904

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/729-artifact-identity-refinements
Nov 6, 2026
Merged

fix(doctor): artifact identity ignores git-ignored files and names trees outside git (#729, slice 2)#904
github-actions[bot] merged 1 commit into
mainfrom
fix/729-artifact-identity-refinements

Conversation

@cdeust

@cdeust cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner

Summary

Refs #729 (tranche 2 : les quatre notes de revue laissées sur le check livré par la tranche 1, #887). Tranches restantes listées sur l'issue.

Cause racine, tracée dans mcp_server/doctor_artifact.py :

  1. tree_members hachait tous les fichiers sous mcp_server/. Un fichier ignoré par git (.DS_Store, ligne 17 du .gitignore) changeait donc le digest de l'arbre, alors que hatchling le laisse hors de la roue. Le digest de l'arbre et celui de la roue construite depuis lui pouvaient différer.
  2. Un arbre sans .git (par exemple un répertoire de plugin copié) échouait avec le conseil git -C … status, qui ne peut rien y faire.
  3. Une roue installée dont le RECORD a disparu était décrite comme un arbre source : l'absence de RECORD était masquée.
  4. Le correctif de réinstallation omettait --python <interpréteur>, que runtime.py setup passe (la règle vient de l'incident du 2026-10-02, Intel CPython choisi par uv sur un hôte ARM).

Correctif.

  1. Dans un checkout, les membres sont ceux que git garde dans l'arbre : git ls-files -z --cached --others --exclude-standard -- mcp_server, suivis ou non suivis mais pas ignorés. Un fichier suivi mais supprimé du disque est sauté. Si ls-files échoue, le check échoue et le dit, comme pour git status.
  2. Sans .git, l'arbre est nommé outside git et passe sur son digest. Git n'est pas appelé.
  3. Une roue localisée à l'emplacement importé sans RECORD fait échouer le check, avec le correctif de réinstallation.
  4. Le correctif nomme --python <sys.executable résolu>.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • pytest tests_py/test_doctor_artifact.py tests_py/test_doctor_artifact_tree.py : 23 passés. Un test utilise un vrai dépôt git dans tmp_path, sans faux : .gitignore avec .DS_Store, un fichier suivi, un fichier non suivi. Le digest couvre 2 fichiers, pas le .DS_Store.
  • pytest tests_py/test_doctor*.py tests_py/handlers/test_check_setup.py tests_py/architecture tests_py/scripts : 2007 passés, 18 ignorés.
  • Porte de mutation ADR-1122 en local (GITHUB_BASE_REF=main python scripts/ci_mutation_diff.py) : 1 file(s), 0 failed, 0 untested, sur 7 fonctions modifiées. Le premier passage avait laissé 11 survivants (optional non vérifié hors git, libellé no git revision, chemin d'une roue dont le RECORD ne hache aucun membre). Les tests ajoutés les tuent tous.
  • Reçu de bout en bout : mcp_server/.DS_Store créé (ignoré), roue construite avec uv build --wheel (0 .DS_Store dans l'archive), installée sans dépendances dans un venv isolé. Roue installée : installed hypermnesia-mcp 4.23.5, 735 mcp_server members, digest d8bf266c…7f8e. Checkout : source tree … 735 mcp_server files, digest d8bf266c…7f8e. Même digest, .DS_Store présent.
  • ruff check et ruff format --check (0.17.3) sur les fichiers modifiés ; python scripts/check_craftsmanship.py : OK.
  • Non vérifié : un vrai répertoire de plugin Claude installé, ni une vraie installation uv tool install (chemin Codex). torch est refusé par le proxy ici, d'où --no-install-package torch. Windows et macOS sont laissés à la CI.

Audit notes

  • Revue : routine autopilot uniquement.
  • Écart connu : --exclude-standard suit aussi .git/info/exclude et le core.excludesFile global, que hatchling ne lit pas. Un fichier ignoré seulement par ces deux sources entrerait dans la roue sans entrer dans le digest de l'arbre. Le test ne couvre que .gitignore.

Coding-standards compliance

  • Couches : doctor_artifact.py n'ajoute que sys (stdlib).
  • 300 lignes par fichier et 40 par méthode : les tests de la tranche 2 sont dans tests_py/test_doctor_artifact_tree.py pour que test_doctor_artifact.py reste sous le plafond.
  • La constante modifiée (_REINSTALL) garde son # source:, mis à jour.

Breaking changes

Aucun. Le check reste optionnel. Un arbre sans .git passe maintenant ; il échouait avant.

Screenshots / logs

[OK  ] Runtime artifact identity   source tree /home/user/Cortex at a5dd8aa0… (version 4.23.5), 735 mcp_server files, digest d8bf266c…7f8e; source evidence, not a distributed artifact

Reviewer checklist

  • CHANGELOG.md mis à jour (### Fixed).
  • Documentation : CHANGELOG et docstring du module.
  • Aucun secret ni donnée personnelle dans le diff.
  • CI verte sur le dernier commit.

Generated by Claude Code

@claude
fix(doctor): artifact identity ignores git-ignored files and names tr…
a58c975 
…ees outside git (#729, slice 2)

Review notes on #887 (slice 1), traced in doctor_artifact.py:
- tree_members hashed every file under mcp_server/, so an ignored
  .DS_Store changed the source-tree digest while hatchling leaves it out
  of the wheel; a checkout now digests git's tree (ls-files -co
  --exclude-standard).
- a tree without .git failed with a `git -C … status` fix that cannot
  help; it is now named 'outside git' and passes on its digest.
- an installed wheel without RECORD was described as a source tree; it
  now fails with the reinstall fix.
- the reinstall fix omitted --python, which runtime.py setup passes.

Refs #729

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYrc9zFgbqiq6bsahve
@cdeust cdeust mentioned this pull request Nov 6, 2026
Open
5 tasks
@cdeust
cdeust marked this pull request as ready for review November 6, 2026 15:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T15:41:52.506448Z a58c975 Draft marked ready
🔒 Security Review ✅ Completed 2026-11-06T15:46:49.703377Z a58c975 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @cdeust's task in 3m 5s —— View job

Review of PR #904 (review 1 of 3) is posted

  • Read the diff
  • Run the tests: not done, tool permissions blocked pytest and ruff in this job. The earlier Pipeline run on this commit is green.
  • Five reviews: code quality, security, performance, test coverage, documentation
  • Post the verdict: review comment

There are no Blockers. The review comment lists three Notes:

  1. A leftover *.egg-info in the checkout would be reported as a wheel with no RECORD.
  2. When ls-files returns nothing, or a wheel's RECORD has no package hashes, the check passes. When RECORD is missing, it fails.
  3. The ls-files output is decoded with the locale encoding, so a non-ASCII file name can drop out of the digest without notice. encoding="utf-8" would fix it.

Verdict: approve

@claude

claude Bot commented Nov 6, 2026

Copy link
Copy Markdown

Review 1 of 3 (ADR-1119), commit a58c975

There are no Blockers. Five reviews ran: code quality, security, performance, test coverage and documentation. I traced each of the four fixes through _identify, _source_tree and tree_members:

  • .git as a file in a worktree is handled.
  • ls-files paths are relative to root.parent.
  • The -z output is parsed correctly.
  • The ls-files failure, the missing RECORD and the tree outside git each fail or pass as described, with a test for each.
  • The _REINSTALL # source: comment matches plugins/hypermnesia-mcp-codex/scripts/runtime.py (--python sys.executable --no-build, the 2026-10-02 docstring note).
  • pyproject.toml has no ignore-vcs setting, so hatchling's .gitignore handling applies.
  • Layers, file and method limits, and # source: on every constant are all respected.
  • The real-git test (git init and add need no identity) follows existing precedent (tests_py/test_doctor.py). Pytest runs only on ubuntu-latest.
  • The earlier Pipeline run on this commit is green.

I could not run pytest or ruff in this job; the tool permissions blocked them.

Notes, no action before merge

  1. mcp_server/doctor_artifact.py:196-203: a leftover hypermnesia_mcp.egg-info/ in the checkout (.gitignore:7) has locate_file("mcp_server") equal to the imported root and no RECORD. With the repo root on sys.path, the check would now fail with "installed … has no RECORD" plus a reinstall fix, where before it described the source tree. Limiting the RECORD failure to .dist-info would remove this.
  2. mcp_server/doctor_artifact.py:159: if ls-files succeeds but prints nothing (a package wholly untracked or ignored), the check passes with "0 mcp_server files". A wheel whose RECORD holds no package hashes also passes, as a source tree, while a missing RECORD fails. These cases could be made consistent.
  3. mcp_server/doctor_artifact.py:149: the ls-files output is decoded in text mode with the locale encoding (subprocess_safe.py:39-40). On a non-UTF-8 locale, a non-ASCII file name would decode wrong, be taken for "deleted from disk" and drop silently out of the digest. Passing encoding="utf-8" fixes it.

Verdict: approve

@cdeust Claude

cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

Les trois notes de la revue 1 restent hors de cette PR : la revue n'a trouvé aucun Blocker, et ADR-1119 ne fait pousser que sur un Blocker. Je les ajoute aux travaux restants de #729, pour une PR d'affinage :

  1. Ne faire échouer le check pour RECORD absent que si la distribution est un .dist-info : un hypermnesia_mcp.egg-info/ resté dans le checkout ne doit pas déclencher l'échec.
  2. Rendre cohérents les cas « 0 fichier listé par ls-files », « RECORD sans hash de paquet » et « RECORD absent ».
  3. Passer encoding="utf-8" aux appels git, pour qu'un nom de fichier non ASCII ne sorte pas du digest sur une locale non UTF-8.

Generated by Claude Code

@cdeust Claude

cdeust commented Nov 6, 2026

Copy link
Copy Markdown
Owner Author

merge en échec sur a58c975, run 42245268852 : le message est environment: abandoned, needs success. Cet échec ne vient pas de la PR.

  • Le job environment (macos-latest) n'a reçu aucun runner (runner_name vide, aucune étape exécutée, aucun log). Il a été annulé après 20 minutes d'attente (15:54Z → 16:15Z).
  • Sur ce même commit, le run précédent a passé environment (macos-latest) au vert. Dans ce run, les jobs environment Ubuntu et Windows, unit, contract, e2e et regression sont verts.
  • Action : une seule relance des jobs en échec (rerun-failed-jobs). Si l'échec persiste avec le même symptôme, la capacité des runners macOS est en cause : le correctif n'est pas dans le code.

Generated by Claude Code

@github-actions
github-actions Bot merged commit b30e690 into main Nov 6, 2026
40 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cdeust @claude