fix(skills): restore bundle/installed content-hash symmetry on Windows

[bbasketballer75]

The contract that was broken

skills_hub.bundle_content_hash() (hashes an in-memory bundle) and skills_guard.content_hash() (hashes the installed directory) both document that they must produce the same digest for the same skill. check_for_skill_updates() relies on it: it compares the lock file's disk-derived content_hash against a freshly fetched bundle_content_hash to decide up_to_date vs update_available.

Three separate defects broke that on Windows.

1. Newline translation in quarantine_bundle()

Text files were written with write_text(content, encoding="utf-8") — newline=None, so universal-newline translation turns every \n into \r\n. That directory is exactly the tree install_from_quarantine() moves into place and then hashes, so installed bytes never equalled bundle bytes for any text file containing a newline.

2. Platform-dependent ordering in _content_digest()

It iterated sorted(skill_path.rglob("*")) — sorting Path objects. WindowsPath compares case-insensitively, so references sorts before SKILL.md on Windows and after it on POSIX. bundle_content_hash sorts relative path strings, so the two disagreed about file order.

3. Native separators in OptionalSkillSource.fetch() keys

Keys came from str(f.relative_to(skill_dir)), producing assets\neutts-cli\samples\jo.wav. Those key strings are themselves hashed by bundle_content_hash, are used as write paths by quarantine_bundle, and are indexed with forward slashes by callers.

User-visible impact

On Windows, every hub-installed skill was permanently reported as update_available, and reinstalling could never clear it — the disk hash was recomputed the same broken way each time.

Defect 2 also makes full_content_hash() non-canonical across platforms, and that value binds scanner attestations and keys the scan cache.

Test change

test_fetch_preserves_binary_assets wrote its sample with write_text("hello\n") and then asserted the bytes returned as b"hello\n". Since the test verifies byte preservation, the fixture has to control its own bytes — it now passes newline="". No assertion was weakened.

Verification

tests/tools/test_skills_hub.py on Windows:

	failed	passed
before	2	57
after	0	59

Newly broken: none — set-diff of failure names is empty. tests/tools/test_skills_guard.py: 31 passed. ruff check clean on all three files.

Reverting the production changes makes test_bundle_content_hash_matches_installed_content_hash fail again, so the coverage pins the behaviour rather than passing vacuously.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Restores the documented contract that an in-memory skill bundle hash (bundle_content_hash) must match the on-disk installed skill hash (content_hash) for the same content, fixing Windows-specific byte/path canonicalization issues that caused hub-installed skills to be perpetually reported as update_available.

Changes:

• Normalize optional-skill bundle file keys to POSIX-style relative paths to avoid Windows separator differences in hashing and lookups.
• Write quarantined text files with newline="" to prevent Windows newline translation from altering installed bytes.
• Canonicalize installed-directory hashing order by sorting relative POSIX path strings (not Path objects) to make digests platform-stable and symmetric with bundle hashing.
• Strengthen tests to exercise the real quarantine writer and ensure byte-fidelity on Windows fixtures.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tools/skills_hub.py	Normalizes bundle file keys and disables newline translation when writing quarantined text files.
tools/skills_guard.py	Makes installed content hashing canonical across platforms by sorting by relative POSIX path strings.
tests/tools/test_skills_hub.py	Updates tests to validate real install-byte fidelity and avoid fixture newline translation on Windows.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bbasketballer75]

Rebased onto current main (the branch was 1334 behind). One conflict, in tools/skills_hub.py.

The conflict resolved itself in your favour

Upstream has since fixed the identifier line independently, and more thoroughly than this PR did:

# this PR
identifier=f"official/{skill_dir.relative_to(self._optional_dir).as_posix()}"
# main today — a strict superset
identifier=f"official/{skill_dir.resolve().relative_to(self._optional_dir.resolve()).as_posix()}"

I took main's line verbatim. The second commit is therefore now comment-only, and I reworded it (docs(skills):) rather than leave a message claiming a normalization it no longer performs. I kept the comment because as_posix() there reads as cosmetic and isn't — it is what stops the lock-file identifier disagreeing with _scan_all()'s and breaking fetch()'s rel.rsplit("/", 1) fallback. Drop that commit if you'd rather not carry the comment; nothing depends on it.

The first commit is still needed

Both of its fixes are absent from main today:

• tools/skills_hub.py:3856 is still file_dest.write_text(file_content, encoding="utf-8") — no newline="", so Python's universal-newline translation rewrites every \n to os.linesep on write. The installed bytes then cannot match the bundle bytes, content_hash(install_dir) never equals bundle_content_hash(bundle), and every text skill is permanently reported as update_available by check_for_skill_updates() — reinstalling cannot clear it, because the reinstall reproduces the same mismatch.
• tools/skills_hub.py:3381 is still rel_path = str(f.relative_to(skill_dir)), which emits assets\x\y.wav on Windows. Those keys are what bundle_content_hash hashes, what quarantine_bundle writes to, and what callers index with forward slashes.

Tests

tests/tools/test_skills_hub.py: 70 passed.

🤖 Rebased by Claude Code