fix(skills): make factorydroid disable-model-invocation consistent with other tools - #1948
Merged
Merged
Conversation
PR #1928 wired a root-level `disable-model-invocation` default into six tools, but factorydroid lacked a per-target section schema, so it could not honor a per-tool override and dropped the value on import — contradicting the documented "any tool section can override" claim. - Add a `factorydroid` section (with `disable-model-invocation`) to the rulesync skill frontmatter schema and input type. - Resolve the value in FactorydroidSkill.fromRulesyncSkill via the shared helper (root default + per-target override) and round-trip it back into the `factorydroid` section in toRulesyncSkill, matching the other five tools. - Extract a shared `resolveDisableModelInvocation` helper and replace the copy-pasted resolution expression in all six tool skill files (claudecode, cursor, zed, pi, qwencode, factorydroid). - Document the `factorydroid` skill section and re-sync skills/rulesync/. - Add unit tests for the helper and for factorydroid per-target override (including the `false`-over-`true` edge case) and import round-trip. Ref: #1928 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
@dyoshikawa Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1931
Ref: #1928
Issue: #1931
PR #1928 added a root-level
disable-model-invocationdefault to the rulesync skill frontmatter and wired it into six tools (claudecode,cursor,zed,pi,qwencode,factorydroid), where each tool's own section value overrides the shared default. Five of those tools have a per-target section schema and round-trip the value, but factorydroid did not: it had nofactorydroid:section in the rulesync skill schema, only read the root value, and dropped the value on import. This contradicted the documented "any tool section can override" claim.Changes
factorydroidsection (with thedisable-model-invocationkey) toRulesyncSkillFrontmatterSchemaInternaland its input type insrc/features/skills/rulesync-skill.ts.FactorydroidSkill.fromRulesyncSkillnow resolves the value via the shared helper (root default + per-target override).FactorydroidSkill.toRulesyncSkillnow round-tripsdisable-model-invocationback into thefactorydroidsection, matching the other five tools. The documented per-target override claim is now true for factorydroid too.resolveDisableModelInvocation({ rootFrontmatter, section })intosrc/features/skills/skills-utils.tsand replaced the copy-pastedsection?.["disable-model-invocation"] ?? rootFrontmatter["disable-model-invocation"]expression in all six tool skill files. Forpi/zed(which spread the section then re-apply the resolved value), behavior is identical and a clarifying comment now makes the spread-then-reapply intent explicit. No observable output change for the five already-correct tools.factorydroid:skill section indocs/reference/file-formats.mdand re-syncedskills/rulesync/file-formats.mdviascripts/sync-skill-docs.ts.resolveDisableModelInvocation.false-overrides-trueedge case) and the import round-trip.Verification
pnpm cicheck(code + content) passes: format, oxlint, typecheck, 6738 tests, skill-doc-sync, cspell, secretlint all green.🤖 Generated with Claude Code