Skip to content

feat(skills): support disable-model-invocation in Claude Code skills#1256

Merged
dyoshikawa merged 2 commits intodyoshikawa:mainfrom
jornetsimon:feat/support-disable-model-invocation-in-skills
Mar 4, 2026
Merged

feat(skills): support disable-model-invocation in Claude Code skills#1256
dyoshikawa merged 2 commits intodyoshikawa:mainfrom
jornetsimon:feat/support-disable-model-invocation-in-skills

Conversation

@jornetsimon
Copy link
Contributor

Add support for the disable-model-invocation frontmatter field in Claude Code skills, matching the existing support in Claude Code commands.

The field is preserved through generate, import, and round-trip conversions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dyoshikawa-claw

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

PR #1256 Review - Support disable-model-invocation in Claude Code Skills

Overall Mergeability Verdict: ✅ MERGEABLE (with minor documentation update needed)


Code Review Summary

Code Quality Score: 8.5/10

  1. ✅ Excellent Test Coverage - 141 new test lines covering true/false values, round-trip conversion, and schema validation with proper type checking.

  2. ✅ Correct Implementation - The disable-model-invocation field properly added to Zod schema with z.optional(z.boolean()) and correctly handles both true and false values using !== undefined checks.

  3. ⚠️ Missing Documentation (MEDIUM) - The new disable-model-invocation field is not documented in docs/reference/file-formats.md. Users won't know this feature exists without documentation.

  4. ✅ Consistency - Follows existing coding guidelines, uses z.looseObject(), kebab-case naming, and static imports appropriately.

  5. ℹ️ Pattern Inconsistency (LOW) - Skills use explicit field listing while commands use simpler spread pattern. This is acceptable for this PR but could be refactored in the future.

  6. ✅ Type Safety - TypeScript types correctly updated in rulesync-skill.ts to include the new field.

  7. ✅ Backward Compatibility - Implementation is backward-compatible and doesn't break existing functionality.

Action Required: Add documentation for disable-model-invocation field to docs/reference/file-formats.md before merging.


Security Review Summary

Security Assessment: ✅ NO ISSUES FOUND

  1. ✅ Input Validation - Uses Zod's strict z.optional(z.boolean()) schema that only accepts true, false, or undefined. Non-boolean values are rejected.

  2. ✅ Data Integrity - Simple boolean field with identity transformation through conversion pipelines. Correctly preserves false values with explicit !== undefined check.

  3. ✅ No Injection Risks - Boolean primitive not interpolated into shell commands, file paths, or executable contexts. Processed by battle-tested gray-matter library.

  4. ✅ Configuration Security - The disable-model-invocation: true setting is defensive/restrictive - can only disable functionality, not enable dangerous operations.

  5. ✅ Schema Validation - Uses z.looseObject() which preserves unknown keys for forward compatibility while strictly validating known fields.

  6. ✅ Type Safety - Proper TypeScript types updated in RulesyncSkillFrontmatterInput with robust runtime Zod validation.

  7. ✅ Existing Controls - PR follows established patterns and doesn't modify existing security controls (path separator injection prevention, path traversal prevention).


Final Recommendation

✅ APPROVED FOR MERGE after documentation update.

The implementation is solid, well-tested, and secure. The only blocker is missing documentation which should be added to docs/reference/file-formats.md before merging. Once documentation is added, this PR is ready to merge.

No security vulnerabilities identified. The code follows best practices and maintains consistency with the existing codebase.

github run

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dyoshikawa dyoshikawa merged commit 8ce082f into dyoshikawa:main Mar 4, 2026
7 checks passed
@github-actions github-actions bot mentioned this pull request Mar 4, 2026
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.

3 participants