Skip to content

fix(skills): invalidate registry cache on content changes - #306

Merged
Alan-TheGentleman merged 1 commit into
Gentleman-Programming:mainfrom
barbatdev:fix/skill-registry-content-fingerprint
Aug 15, 2026
Merged

Alan-TheGentleman merged 1 commit into
Gentleman-Programming:mainfrom
barbatdev:fix/skill-registry-content-fingerprint

Conversation

@barbatdev

@barbatdev barbatdev commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Include each indexed SKILL.md content hash in the registry fingerprint.
  • Force a one-time cache refresh by advancing the registry schema to version 7.
  • Preserve deterministic ordering and best-effort missing/unreadable file behavior.

Advances #256 (track 7 of 10, skill fingerprint slice). Does not close #256.

Review path

  1. Review extensions/skill-registry.ts for the single-cache content fingerprint and schema bump.
  2. Review tests/skill-registry.test.ts for the same-path, same-size, restored-mtime regression.

Verification

  • RED: old code returned a cache hit with changed bytes and restored metadata
  • Skill registry and collision-prefix tests: 30/30
  • Package structural tests: 40/40
  • git diff --check: passed
  • Native reliability review: no findings
  • Native pre-commit and pre-push gates: allow

Scope

This PR covers only the content-fingerprint slice of issue #256 track 7. The delegated Key Learnings closing contract remains a separate rollback boundary and is intentionally out of scope.

Summary by CodeRabbit

  • Bug Fixes
    • Improved skill registry refresh detection by identifying content changes even when file size and modification time remain unchanged.
    • Ensured regenerated registries reflect the latest skill content and exclude outdated entries.
    • Improved handling and tracking of unreadable skill files.

@barbatdev barbatdev added the type:bug Bug fix label Aug 12, 2026
@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ac195f7f-d9cf-4040-bf83-6ba2c0a3292b

📥 Commits

Reviewing files that changed from the base of the PR and between 19b0ed7 and 047a437.

📒 Files selected for processing (2)
  • extensions/skill-registry.ts
  • tests/skill-registry.test.ts

📝 Walkthrough

Walkthrough

The registry schema advances to version 7. Skill fingerprints now include SHA-1 content hashes and record unreadable files. Tests verify regeneration after same-size rewrites that preserve modification times.

Changes

Skill registry fingerprinting

Layer / File(s) Summary
Content fingerprint and regeneration wiring
extensions/skill-registry.ts
The registry version changes to 7. Fingerprints include skill content hashes. Unreadable files are recorded. regenerateRegistry is exposed through __testing.
Metadata-preserving rewrite regression coverage
tests/skill-registry.test.ts
The test rewrites skill content without changing file size or modification time. It verifies fingerprint-based regeneration and updated registry contents.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies registry cache invalidation caused by skill content changes.
Linked Issues check ✅ Passed The changes implement the content-byte fingerprint portion of issue #256, including schema refresh, unreadable-file handling, and regression coverage.
Out of Scope Changes check ✅ Passed The registry changes and regression tests remain within the content-fingerprint scope described for issue #256.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Alan-TheGentleman
Alan-TheGentleman merged commit 6073948 into Gentleman-Programming:main Aug 15, 2026
2 checks passed
@Alan-TheGentleman

Copy link
Copy Markdown
Collaborator

Merged. Verified the RED empirically: with only the regenerateRegistry export on baseline, the new test fails (regenerated: false), and your version passes — a real false negative when content changes under an unchanged (path, size, mtime) triple.

Measured cost of the content hash on the previously stat-only cache-hit path: ~2.1 ms → ~12.1 ms over 194 real skill files (1.2 MB), per session start and per watcher debounce. Worth it for the correctness.

Two optional follow-ups if you want them (not blocking, deliberately left as-is):

  1. Now that content is hashed, ${file}:${mtimeMs}:${size}:${contentHash} could collapse to ${file}:${contentHash} — today a bare touch or a git checkout (which resets mtimes) still forces a full regeneration.
  2. The new test calls regenerateRegistry, which scans the real userSkillDirs(); an injectable home root would make the guard airtight against a concurrent $HOME skill write in the window between the two calls.

Thanks 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(parity): adopt Gentle AI post-v2.2.2 behavioral parity

2 participants