Skip to content

fix: root-level skill update breaks directory structure and fails backfill#56

Merged
crossoverJie merged 1 commit into
mainfrom
fix/root-level-skill-update
Jun 25, 2026
Merged

fix: root-level skill update breaks directory structure and fails backfill#56
crossoverJie merged 1 commit into
mainfrom
fix/root-level-skill-update

Conversation

@crossoverJie

Copy link
Copy Markdown
Owner

Problem

When updating a root-level skill (SKILL.md directly in the repo root, e.g. eze-is/web-access), two bugs occur:

  1. Directory becomes a file: updateSkill() derives folderPath = "SKILL.md" instead of "", so copyItem copies the SKILL.md file over the skill directory, breaking the symlink. Claude Code can no longer find the skill.

  2. Update detection fails: checkForUpdate() and checkAllUpdates() use the same buggy folderPath derivation, causing getTreeHash to compute git rev-parse HEAD:SKILL.md (blob hash) instead of git rev-parse HEAD: (root tree hash). Hash comparison produces incorrect results.

  3. Backfill crashes: findCommitForTreeHash with empty folderPath runs git log --format=%H -- "", which git rejects as an invalid empty pathspec.

Root Cause

The lock file stores skillPath = "SKILL.md" for root-level skills. The inline folderPath derivation logic checked hasSuffix("/SKILL.md")"SKILL.md" does not end with "/SKILL.md", so it fell through to folderPath = lockEntry.skillPath (= "SKILL.md"), instead of being treated as an empty path.

Fix

  • Extracted deriveFolderPath(from:) static helper in SkillManager — handles "SKILL.md""" explicitly, eliminating duplicated logic across checkForUpdate(), checkAllUpdates(), and updateSkill()
  • Fixed findCommitForTreeHash in GitService — when folderPath is empty, omits the -- separator to avoid the empty pathspec error
  • Added 10 unit tests in RootLevelSkillUpdateTests.swift covering derivation logic, hash behavior, directory preservation, and backfill regression

Manual Verification Required

  • Install a root-level skill (e.g. eze-is/web-access) via SkillDeck, verify it appears in Claude Code
  • Trigger an update for a root-level skill, verify the directory structure is preserved (not flattened to a file)
  • Verify sub-directory skills (e.g. from vercel-labs/skills) still update correctly — no regression

Regression Checklist

  • Sub-directory skill install/update still works (e.g. find-skills, systematic-debugging)
  • Update detection (checkmark badges) still shows correct status for GitHub-backed skills
  • Batch "Check All Updates" still works for mixed root-level and sub-directory skills
  • Local import skills are unaffected (no lock entry with sourceType: "github")
  • ClawHub-installed skills are unaffected

…kfill

- Root cause: when skillPath is "SKILL.md" (root-level skill), the old
  code in checkForUpdate/checkAllUpdates/updateSkill set folderPath to
  "SKILL.md" instead of "". This caused two problems:
  1. getTreeHash("SKILL.md") returned blob hash instead of root tree hash,
     breaking update detection hash comparison
  2. updateSkill copied the SKILL.md file over the skill directory,
     turning the directory into a file and breaking symlinks
- Extracted deriveFolderPath(from:) helper in SkillManager to eliminate
  the duplicated folderPath derivation logic across three methods
- Fixed findCommitForTreeHash in GitService: empty folderPath caused
  "git log --format=%H -- """ to fail with empty pathspec error; now
  omits the "--" separator for root-level skills
- Added 10 unit tests in RootLevelSkillUpdateTests covering:
  - deriveFolderPath logic for root-level and sub-directory skills
  - Regression test ensuring old bug does not return
  - getTreeHash behavior difference (root tree vs blob hash)
  - Directory structure preservation after update
  - findCommitForTreeHash backfill for root-level skills

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@crossoverJie
crossoverJie merged commit ab9e628 into main Jun 25, 2026
1 check passed
@crossoverJie
crossoverJie deleted the fix/root-level-skill-update branch June 25, 2026 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant