SIGN IN SIGN UP

feat(skill-loader): honor disable-model-invocation frontmatter (#62)

* feat(skill-loader): honor disable-model-invocation frontmatter

Implements the Claude Code spec field 'disable-model-invocation'. When
set to true in a SKILL.md frontmatter, the skill is blocked from both
model-initiated invocation paths:

1. skill(name=...) tool - throws with an explicit error pointing the
   model at the slash-command menu.
2. task(load_skills=[...]) delegation - the resolver refuses the whole
   batch if any requested skill has the flag set, naming the blocked
   skill in the error.

Slash-command invocation remains available to the user (that path is
gated by the separate 'user-invocable' field, unchanged).

Changes:
- LoadedSkill.disableModelInvocation: raw frontmatter value, surfaced
  by loadSkillFromPath from data['disable-model-invocation'].
- Skill tool (src/tools/skill/tools.ts): early-throw if the matched
  skill has disableModelInvocation: true, before permission prompt
  or body extraction.
- Task resolver (src/tools/delegate-task/skill-resolver.ts): lookup
  the flag via discoverSkills; reject the batch atomically rather
  than silently dropping blocked skills.

Tests:
- skill-directory-loader.test.ts: loader reads the frontmatter field
  in all three states (true/false/unset) matching the user-invocable
  test pattern.
- disable-model-invocation-gate.test.ts: skill tool blocks true,
  allows false and unset, names the specific skill in errors.
- skill-resolver-disable-model-invocation.test.ts: task resolver
  blocks true, allows false and unset, rejects mixed batches,
  bypasses the getAllSkills cache via sentinel disabledSkills to
  avoid cross-file test pollution.

Verification:
- bun run typecheck: clean
- bun test: 5640 pass, 0 fail (5602 -> 5640; +38 from new tests
  and expect calls)
- lsp_diagnostics on src/: 0 errors (pre-existing bun:test LSP
  warning is environmental, not introduced by this change)

Closes the handoff §C.1 blocker - disable-model-invocation was
already parsed into SkillMetadata but never surfaced to LoadedSkill
or gated at runtime.

* fix(skill-loader): use getAllSkills for gate + case-insensitive + honest errors

Addresses reviewer feedback on PR #62:

1. Gate now uses getAllSkills(options) instead of bare discoverSkills
   (Copilot, codex, cubic)
   Before: discoverSkills skipped options.disabledSkills and
   options.browserProvider filtering. A skill explicitly disabled in
   config AND marked disable-model-invocation: true would be rejected
   with the frontmatter error instead of treated as unavailable. Same
   risk for provider-gated skills (agent-browser vs playwright).

   After: getAllSkills(options) applies both filters and is cached, so
   the gate sees the same filtered skill set as the downstream
   resolveMultipleSkillsAsync call. No more filter-bypass confusion.

2. Single filesystem scan (devin, cubic)
   Before: resolveSkillContent called discoverSkills then
   resolveMultipleSkillsAsync (which internally calls getAllSkills
   which also calls discoverSkills). Two filesystem traversals per
   request.

   After: getAllSkills at the top is cached. resolveMultipleSkillsAsync
   hits the same cache. One scan per cache lifetime.

3. Case-insensitive skill name matching (kilo-code-bot)
   Before: gate filter used strict === comparison. skill tool uses
   matchSkillByName which is case-insensitive (via skill-matcher.ts).
   task(load_skills=['My-Skill']) would miss a skill defined as
   'my-skill', silently bypassing the gate.

   After: build a name-lowercase map, check with name.toLowerCase().
   Behavioral parity between the two gating paths.

4. Reworded error to not promise slash availability (Copilot, cubic)
   Before: 'can only be invoked by the user via the slash-command menu'
   promises slash access even when user-invocable: false also blocks
   slash.

   After: 'Model-initiated invocation is blocked. If a skill is also
   user-invocable, the user can still trigger it from the slash-command
   menu.' - states the invariant without promising the fallback.

   Same rewording applied to the skill tool error in tools.ts and to
   the LoadedSkill.disableModelInvocation docstring.

Test coverage added:
- Case-insensitive gate match regression test
- Error-wording regression test for the combined user-invocable: false +
  disable-model-invocation: true scenario

Verification:
- bun run typecheck: clean
- bun test: 5642 pass, 0 fail (5640 -> 5642, +2 tests)
- lsp_diagnostics on src/: no new errors
V
Vacbo committed
086a8a7b2675377b1af481261a4d22f27601dbec
Parent: f56445c
Committed by GitHub <noreply@github.com> on 4/22/2026, 2:52:48 AM