fix(skills): normalize path separators and sort order for cross-platform hash parity - #71252
fix(skills): normalize path separators and sort order for cross-platform hash parity#71252ForeverAfter wants to merge 1 commit into
Conversation
…orm hash parity content_hash (disk) and bundle_content_hash (in-memory bundle) must be symmetric, but on Windows they diverged for skills with subdirectories, causing a perpetual update_available loop (NousResearch#71237): - OptionalSkillSource.fetch() built bundle keys with str(relative_to()), yielding backslash-separated paths on Windows while the disk digest uses Path.as_posix(). - bundle_content_hash() hashed rel paths verbatim, so backslash-keyed bundles never matched the posix-normalized disk digest. - _content_digest() iterated sorted(Path.rglob()), ordering by Path components, while bundle_content_hash sorts posix strings — a different file order and therefore a different SHA-256 even with identical separators and content. Normalize bundle keys to posix, normalize separators defensively in bundle_content_hash, and sort the disk digest by posix strings so both sides iterate identically on every platform. Adds regression tests asserting disk/bundle hash parity for nested and flat skills and for backslash-keyed bundles. Fixes NousResearch#71237 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Independent verification on current A deterministic Linux reproducer with a file/directory prefix collision produces different clean-main hashes (
The path-dependent mismatch remains because
A complete fix is to reuse canonical_files = {}
for raw_path, content in bundle.files.items():
canonical = _normalize_bundle_path(
raw_path,
field_name="bundle file path",
allow_nested=True,
)
canonical_files[canonical] = content
for canonical in sorted(
canonical_files,
key=lambda path: PurePosixPath(path).parts,
):
...Validation of that variant:
This keeps validation and normalization semantics in one helper and fixes both separator and ordering parity without hashing a tree different from the one actually installed. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the disk/bundle hash symmetry issue. The current-main premise is valid: tools/skills_guard.py:695-699 orders disk entries as Path objects, while tools/skills_hub.py:3604-3614 orders raw bundle strings and OptionalSkillSource.fetch still emits native-string paths at tools/skills_hub.py:3131-3140.
Problems
- The added
norm_path = rel_path.replace("\\", "/")occurs aftersorted(bundle.files). It therefore does not make ordering canonical. For example, rawa-zanda\\bsort differently from the disk-side component order aftera\\bbecomesa/b. - The hash must reflect what installation writes.
quarantine_bundlecalls_normalize_bundle_pathbefore writing attools/skills_hub.py:3464-3479; the PR does not apply that normalization before hashing, so whitespace and canonical aliases can still diverge from the installed tree.
Suggested changes
- Normalize keys with
_normalize_bundle_path, collapse canonical duplicates with installer-equivalent last-write-wins behavior, and use one explicit component-order key in both hash functions. - Add regressions for the ordering, alias, and whitespace cases above.
Automated hermes-sweeper review.
| # platforms. Without this, Windows bundles produce a different digest | ||
| # than the installed files, causing a perpetual "update_available". | ||
| norm_path = rel_path.replace("\\", "/") | ||
| h.update(norm_path.encode("utf-8")) |
There was a problem hiding this comment.
This normalizes only after raw-key sorting. For raw keys a-z and a\\b, the bundle can hash a-z before normalized a/b, while disk Path-component ordering puts a/b first. Normalize/canonicalize before sorting, then use the same explicit component-order key on both hash paths.
Problem
Follow-up to #71246 — that PR fixes one of the three path/hash mismatches that cause the perpetual
update_availableloop on Windows (#71237). This PR fixes the remaining two so the loop is fully resolved.Even with #71246 applied (backslash → posix in
OptionalSkillSource.fetch), skills with subdirectories still loop on Windows because of two additional inconsistencies betweenbundle_content_hashand_content_digest:bundle_content_hashdoes not normalize separators. Any bundle whose file keys still carry backslashes (from a Windows-built bundle, a GitHub source on Windows, or a stale lock entry) diverges from the posix-normalized disk digest._content_digestsortsPathobjects, not strings.sorted(skill_path.rglob("*"))orders by path components — on NTFS this places subdirectory files before root-level files (e.g.references/cli.mdbeforeSKILL.md).bundle_content_hashsorts the posix strings alphabetically, whereSKILL.mdcomes first. Different iteration order → different SHA-256 → permanent mismatch, even with identical separators and identical file content.Reproduction (Windows, even with #71246 applied)
Observed hashes on the same skill, same content:
_content_digest(disk, sorted byPath)sha256:1f1c139f07f16489bundle_content_hash(bundle, backslash keys)sha256:0ae3a5fa4ec70721\instead of/sha256:f6d7c9ac8e6a55a0Fix
Three changes (two files). Change 1 is identical to #71246 and is included so this PR is self-contained and can merge independently.
1.
tools/skills_hub.py—OptionalSkillSource.fetch(): posix keys2.
tools/skills_hub.py—bundle_content_hash(): normalize backslashesDefense-in-depth normalization: even if a bundle's file keys arrive with backslashes (GitHub source on Windows, hand-crafted bundles, or future sources), the hash is computed on the posix form, matching what
_content_digestproduces on disk.3.
tools/skills_guard.py—_content_digest(): sort as posix stringssorted(Path)comparesPathobjects component-by-component; combined withrglobtraversal this yields a different file order thansorted(str)on posix-joined paths. Collecting to strings first and then sorting makes_content_digestandbundle_content_hashiterate in the same deterministic order on every platform. The docstring ofcontent_hashalready requires the two functions to stay symmetric — this restores that invariant on Windows.Tests
Adds
tests/tools/test_skills_hash_parity.pywith three regression tests:test_disk_hash_matches_bundle_hash_with_subdirectories— builds a skill withreferences/andscripts/subdirs on disk, assertscontent_hash(dir) == bundle_content_hash(bundle)on all platforms (catches the sort-order bug).test_bundle_hash_normalizes_backslash_keys— asserts a backslash-keyed bundle hashes identically to the posix-keyed equivalent and to the disk hash (catches the separator bug).test_flat_skill_still_matches— flat single-file skills never regressed; keeps it that way.After the fix (verified on Windows 11, v0.19.0)
Relationship to #71246
str()→.as_posix()infetch()).Fixes #71237
🤖 Generated with Claude Code