Skip to content

feat: prompt user to rescan when skill folder changes detected - #15

Open
dominic323 wants to merge 1 commit into
Evol-ai:mainfrom
dominic323:feature/auto-detect-skill-changes
Open

feat: prompt user to rescan when skill folder changes detected#15
dominic323 wants to merge 1 commit into
Evol-ai:mainfrom
dominic323:feature/auto-detect-skill-changes

Conversation

@dominic323

@dominic323 dominic323 commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Adds filesystem sync check in smart entry to detect when skills are manually installed or removed. Prompts user to rescan inventory instead of silently using stale cache.

  • Compares cached inventory count with actual ~/.claude/skills/ files
  • Prompts user with clear options when mismatch detected
  • Respects user choice to skip rescan if desired
  • Adds ~50ms overhead for Glob check (acceptable for UX improvement)

Fixes issue where manually installed skills were invisible until user explicitly mentioned them.

Summary

Adds filesystem sync check to detect when skills are manually installed or removed, prompting the user to rescan inventory instead of silently using stale cache.

Problem

When users manually install a new skill (e.g., git clone skill-repo ~/.claude/skills/new-skill), running /skillcompass does not detect the change. The tool trusts the cached inventory in setup-state.json without verifying it against the filesystem, causing newly installed skills to be invisible.

Solution

Modified the smart entry logic (Step 3 in SKILL.md) to:

  1. Use Glob tool to count actual ~/.claude/skills/*/SKILL.md files
  2. Compare with cached inventory.length
  3. If mismatch detected, prompt user: "Detected N new skill(s). Rescan inventory now?"
  4. Respect user choice — only rescan if they confirm

Changes

  • File: SKILL.md (lines 126-137, inserted filesystem sync check)
  • Performance: Adds ~50ms overhead for Glob check (acceptable)
  • UX: Follows SkillCompass pattern "Choices, not raw commands"

Why prompt instead of auto-scan?

  • Respects user agency (they may have intentionally excluded certain skills)
  • Follows existing interaction conventions
  • Allows users to defer rescan if in the middle of other work

Test Plan

  • Install new skill manually → run /skillcompass → should prompt to rescan
  • Remove skill manually → run /skillcompass → should prompt to update
  • No changes → run /skillcompass → should skip check and continue normally
  • User declines rescan → should continue with cached inventory
  • User accepts rescan → should update setup-state.json

Related Issue

Discovered during user session where agile-product-owner was manually installed but not detected until user explicitly mentioned it.

Adds filesystem sync check in smart entry to detect when skills are
manually installed or removed. Prompts user to rescan inventory
instead of silently using stale cache.

- Compares cached inventory count with actual ~/.claude/skills/ files
- Prompts user with clear options when mismatch detected
- Respects user choice to skip rescan if desired
- Adds ~50ms overhead for Glob check (acceptable for UX improvement)

Fixes issue where manually installed skills were invisible until
user explicitly mentioned them.
@dominic323
dominic323 requested a review from krishna-505 as a code owner April 14, 2026 08:23
@krishna-505

krishna-505 commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Thanks for picking this up — the underlying UX problem is real (silent cache drift when skills are manually installed or removed is a genuine gap).

That said, I don't think this is ready to merge as-is. The core issue isn't where the check lives; it's that the comparison model doesn't match the existing discovery contract.

Main blocker: inventory.length is not comparable to Glob("~/.claude/skills/*/SKILL.md").length

inventory is the result of a canonicalized multi-root scan, not a raw file count. Per commands/setup.md:

  • L36 — discovery scans five roots in priority order: .claude/skills/, .openclaw/skills/, skills.load.extraDirs from ~/.openclaw/openclaw.json, ~/.claude/skills/, ~/.openclaw/skills/. This PR only globs the fourth.
  • L46 — each root is scanned for both */SKILL.md and */skill.md. Lowercase files are silently dropped here.
  • L54–69packages are added to inventory with type: "package", and they can exist without a top-level SKILL.md (e.g. a package whose entry is .claude-plugin/ + sub-skills). The glob can't see them.
  • L111 — entries are deduplicated by canonical identity (project-level wins over user-level for the same name). The raw file count and the deduped inventory length aren't expected to match on a healthy machine.

Net effect: any user with project-level skills, OpenClaw skills, a package without a root SKILL.md, or a skill present at both project and user level will hit a false "drift detected" prompt on every no-args /skillcompass call. That's the main blocker for me — the rule is structurally inconsistent with how inventory is built.

Secondary concerns (not individually blocking)

  • Hot path. SKILL.md L123 smart entry is the high-frequency no-args entrypoint; its job is a fast decision between inbox view and summary. An AskUserQuestion before that is a noticeable UX regression, and there's no per-session throttle — declining once doesn't stick across invocations.
  • Underspecified action. "run inventory rescan (same logic as /setup command)" isn't a defined runtime action. Does Claude Read commands/setup.md inline, or dispatch /setup as a subcommand? Those behave differently (return path, state writes, auto-trigger mode). Worth nailing down.
  • Resume flow. After rescan, does control return to the smart-entry flow (inbox check, summary) or exit? Not specified.

Suggested direction

  1. Fix the comparison model first. Compare canonical identity sets derived from the same discovery contract in commands/setup.md — all five roots, both filename variants, package entries, and the same dedup priority/order. Anything cheaper will keep misfiring.
  2. Move it off the hot path. The SessionStart path in hooks/scripts/session-tracker.js already loads setup-state.json and injects additionalContext. That's one natural home — once-per-session, and the drift notice can surface via the existing context channel rather than a synchronous prompt. Other options work too (a debounced check inside commands/setup.md, a lib/ helper with a short-TTL cache); the criterion is just "don't gate every /skillcompass call".
  3. Persist the user's choice. If drift is detected and the user declines, record that under .skill-compass/cc/ so the next call doesn't re-prompt with the same message.

Happy to look at a revised version.

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.

2 participants