From 5c6aa64e9c017eaa50be97673319409ba0b8fbbf Mon Sep 17 00:00:00 2001 From: Wondr Date: Fri, 5 Jun 2026 11:03:06 +0100 Subject: [PATCH] fix: allow Claude skills to opt out of model invocation --- src/specify_cli/agents.py | 25 ++- src/specify_cli/commands/init.py | 4 + src/specify_cli/extensions.py | 6 +- src/specify_cli/integrations/_helpers.py | 24 ++- .../integrations/_install_commands.py | 8 +- .../integrations/_migrate_commands.py | 8 +- src/specify_cli/integrations/agy/__init__.py | 11 +- src/specify_cli/integrations/base.py | 11 +- .../integrations/claude/__init__.py | 63 ++++++- .../integrations/copilot/__init__.py | 11 +- .../integrations/hermes/__init__.py | 5 +- src/specify_cli/integrations/vibe/__init__.py | 11 +- src/specify_cli/presets.py | 27 ++- tests/integrations/test_integration_claude.py | 165 +++++++++++++++++- tests/test_extension_skills.py | 22 +++ 15 files changed, 378 insertions(+), 23 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 3c06418014..58966aeefe 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -335,7 +335,30 @@ def render_skill_command( description, f"{source_id}:{source_file}", ) - return self.render_frontmatter(skill_frontmatter) + "\n" + body + skill_content = self.render_frontmatter(skill_frontmatter) + "\n" + body + + from . import load_init_options + from .integrations import get_integration + + init_options = load_init_options(project_root) + parsed_options = ( + init_options.get("integration_parsed_options") + if isinstance(init_options, dict) + else None + ) + if not isinstance(parsed_options, dict): + parsed_options = None + integration = get_integration(agent_name) + if integration is not None and hasattr( + integration, + "post_process_skill_content", + ): + skill_content = integration.post_process_skill_content( + skill_content, + parsed_options=parsed_options, + ) + + return skill_content @staticmethod def build_skill_frontmatter( diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index 68f5bed31f..003b935fde 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -459,6 +459,10 @@ def init( from ..integrations.base import SkillsIntegration as _SkillsPersist if isinstance(resolved_integration, _SkillsPersist) or getattr(resolved_integration, "_skills_mode", False): init_opts["ai_skills"] = True + if integration_options: + init_opts["integration_options"] = integration_options + if integration_parsed_options: + init_opts["integration_parsed_options"] = integration_parsed_options save_init_options(project_path, init_opts) # --- agent-context extension (bundled, auto-installed) --- diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index bddf637cbc..94bd94c649 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -917,6 +917,9 @@ def _register_extension_skills( selected_ai = opts.get("ai") if not isinstance(selected_ai, str) or not selected_ai: return [] + integration_parsed_options = opts.get("integration_parsed_options") + if not isinstance(integration_parsed_options, dict): + integration_parsed_options = None registrar = CommandRegistrar() integration = get_integration(selected_ai) @@ -1009,7 +1012,8 @@ def _register_extension_skills( ) if integration is not None and hasattr(integration, "post_process_skill_content"): skill_content = integration.post_process_skill_content( - skill_content + skill_content, + parsed_options=integration_parsed_options, ) if link_outputs: diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index a95f36563a..fe02cb1a0c 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -22,6 +22,8 @@ write_integration_json as _write_integration_json_file, ) +_UNSET = object() + def _get_speckit_version() -> str: """Return the current Spec Kit version. @@ -273,6 +275,8 @@ def _update_init_options_for_integration( project_root: Path, integration: Any, script_type: str | None = None, + raw_options: str | None | object = _UNSET, + parsed_options: dict[str, Any] | None | object = _UNSET, ) -> None: """Update init-options.json and the agent-context extension config to reflect *integration* as the active one. @@ -305,6 +309,18 @@ def _update_init_options_for_integration( else: opts.pop("ai_skills", None) + if raw_options is not _UNSET: + if isinstance(raw_options, str) and raw_options: + opts["integration_options"] = raw_options + else: + opts.pop("integration_options", None) + + if parsed_options is not _UNSET: + if isinstance(parsed_options, dict) and parsed_options: + opts["integration_parsed_options"] = parsed_options + else: + opts.pop("integration_parsed_options", None) + # Update the agent-context extension config BEFORE init-options.json # so a failure here doesn't leave init-options partially updated. ext_cfg_path = project_root / _AGENT_CTX_EXT_CONFIG @@ -374,7 +390,13 @@ def _set_default_integration( ) from exc _write_integration_json(project_root, key, installed_keys, settings) - _update_init_options_for_integration(project_root, integration, script_type=resolved_script) + _update_init_options_for_integration( + project_root, + integration, + script_type=resolved_script, + raw_options=raw_options, + parsed_options=parsed_options, + ) def _set_default_integration_or_exit(*args: Any, **kwargs: Any) -> None: diff --git a/src/specify_cli/integrations/_install_commands.py b/src/specify_cli/integrations/_install_commands.py index 66fd2b2d26..931444e883 100644 --- a/src/specify_cli/integrations/_install_commands.py +++ b/src/specify_cli/integrations/_install_commands.py @@ -158,7 +158,13 @@ def integration_install( ) _write_integration_json(project_root, new_default, new_installed, settings) if new_default == integration.key: - _update_init_options_for_integration(project_root, integration, script_type=selected_script) + _update_init_options_for_integration( + project_root, + integration, + script_type=selected_script, + raw_options=raw_options, + parsed_options=parsed_options, + ) else: _refresh_init_options_speckit_version(project_root) diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index 01cb51d687..b0d3979b25 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -464,7 +464,13 @@ def integration_upgrade( new_manifest.save() _write_integration_json(project_root, installed_key, installed_keys, settings) if installed_key == key: - _update_init_options_for_integration(project_root, integration, script_type=selected_script) + _update_init_options_for_integration( + project_root, + integration, + script_type=selected_script, + raw_options=raw_options, + parsed_options=parsed_options, + ) else: _refresh_init_options_speckit_version(project_root) except Exception as exc: diff --git a/src/specify_cli/integrations/agy/__init__.py b/src/specify_cli/integrations/agy/__init__.py index 6ed69e1e0e..45a23bb26b 100644 --- a/src/specify_cli/integrations/agy/__init__.py +++ b/src/specify_cli/integrations/agy/__init__.py @@ -78,7 +78,11 @@ def repl(m: re.Match[str]) -> str: content, ) - def post_process_skill_content(self, content: str) -> str: + def post_process_skill_content( + self, + content: str, + parsed_options: dict[str, Any] | None = None, + ) -> str: """Inject the dot-to-hyphen hook command note.""" return self._inject_hook_command_note(content) @@ -119,7 +123,10 @@ def setup( continue content = path.read_bytes().decode("utf-8") - updated = self.post_process_skill_content(content) + updated = self.post_process_skill_content( + content, + parsed_options=parsed_options, + ) if updated != content: path.write_bytes(updated.encode("utf-8")) self.record_file_in_manifest(path, project_root, manifest) diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index def5ad20ba..814e3adf97 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -1644,7 +1644,11 @@ def repl(m: re.Match[str]) -> str: content, ) - def post_process_skill_content(self, content: str) -> str: + def post_process_skill_content( + self, + content: str, + parsed_options: dict[str, Any] | None = None, + ) -> str: """Post-process a SKILL.md file's content after generation. Called by external skill generators (presets, extensions) to let @@ -1756,7 +1760,10 @@ def _quote(v: str) -> str: f"{processed_body}" ) - skill_content = self.post_process_skill_content(skill_content) + skill_content = self.post_process_skill_content( + skill_content, + parsed_options=parsed_options, + ) # Write speckit-/SKILL.md skill_dir = skills_dir / skill_name diff --git a/src/specify_cli/integrations/claude/__init__.py b/src/specify_cli/integrations/claude/__init__.py index 57ecb354ad..b97978c1d1 100644 --- a/src/specify_cli/integrations/claude/__init__.py +++ b/src/specify_cli/integrations/claude/__init__.py @@ -7,7 +7,7 @@ import yaml -from ..base import SkillsIntegration +from ..base import IntegrationOption, SkillsIntegration from ..manifest import IntegrationManifest # Mapping of command template stem → argument-hint text shown inline @@ -45,6 +45,17 @@ class ClaudeIntegration(SkillsIntegration): context_file = "CLAUDE.md" multi_install_safe = True + @classmethod + def options(cls) -> list[IntegrationOption]: + return [ + IntegrationOption( + "--no-model-invocation", + is_flag=True, + default=False, + help="Set generated Claude skills to user-invocable only", + ), + ] + @staticmethod def inject_argument_hint(content: str, hint: str) -> str: """Insert ``argument-hint`` after the first ``description:`` in YAML frontmatter. @@ -149,11 +160,55 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str out.append(line) return "".join(out) - def post_process_skill_content(self, content: str) -> str: + @staticmethod + def _set_frontmatter_flag(content: str, key: str, value: str) -> str: + """Set ``key: value`` in frontmatter, inserting it when missing.""" + lines = content.splitlines(keepends=True) + out: list[str] = [] + dash_count = 0 + replaced = False + + for line in lines: + stripped = line.rstrip("\n\r") + if stripped == "---": + dash_count += 1 + out.append(line) + continue + if dash_count == 1 and stripped.startswith(f"{key}:"): + if line.endswith("\r\n"): + eol = "\r\n" + elif line.endswith("\n"): + eol = "\n" + else: + eol = "" + out.append(f"{key}: {value}{eol}") + replaced = True + continue + out.append(line) + + if replaced: + return "".join(out) + return ClaudeIntegration._inject_frontmatter_flag(content, key, value) + + def post_process_skill_content( + self, + content: str, + parsed_options: dict[str, Any] | None = None, + ) -> str: """Inject Claude-specific frontmatter flags and hook notes.""" - updated = super().post_process_skill_content(content) + updated = super().post_process_skill_content( + content, + parsed_options=parsed_options, + ) updated = self._inject_frontmatter_flag(updated, "user-invocable") - updated = self._inject_frontmatter_flag(updated, "disable-model-invocation", "false") + disable_model_invocation = bool( + parsed_options and parsed_options.get("no_model_invocation") + ) + updated = self._set_frontmatter_flag( + updated, + "disable-model-invocation", + "true" if disable_model_invocation else "false", + ) return updated def setup( diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 6e3daeefeb..42fd04d639 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -282,7 +282,11 @@ def command_filename(self, template_name: str) -> str: """Copilot commands use ``.agent.md`` extension.""" return f"speckit.{template_name}.agent.md" - def post_process_skill_content(self, content: str) -> str: + def post_process_skill_content( + self, + content: str, + parsed_options: dict[str, Any] | None = None, + ) -> str: """Inject shared hook guidance into Copilot skill content. Delegates to :class:`_CopilotSkillsHelper` for shared post-processing. @@ -413,7 +417,10 @@ def _setup_skills( continue content = path.read_text(encoding="utf-8") - updated = self.post_process_skill_content(content) + updated = self.post_process_skill_content( + content, + parsed_options=parsed_options, + ) if updated != content: path.write_bytes(updated.encode("utf-8")) self.record_file_in_manifest(path, project_root, manifest) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 1e739002fc..9721a4b742 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -172,7 +172,10 @@ def _quote(v: str) -> str: f"{processed_body}" ) - skill_content = self.post_process_skill_content(skill_content) + skill_content = self.post_process_skill_content( + skill_content, + parsed_options=parsed_options, + ) # Write directly to global ~/.hermes/skills/speckit-/SKILL.md skill_dir = global_skills_dir / skill_name diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index 7922aa8418..43906cded2 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -81,12 +81,19 @@ def _inject_frontmatter_flag(content: str, key: str, value: str = "true") -> str out.append(line) return "".join(out) - def post_process_skill_content(self, content: str) -> str: + def post_process_skill_content( + self, + content: str, + parsed_options: dict[str, Any] | None = None, + ) -> str: """ Inject shared hook guidance and Vibe-specific frontmatter flags: - user-invocable: allows the skill to be invoked by the user (not just other agents) """ - updated = super().post_process_skill_content(content) + updated = super().post_process_skill_content( + content, + parsed_options=parsed_options, + ) updated = self._inject_frontmatter_flag(updated, "user-invocable") return updated diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 47b4240d85..b55163664b 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1056,6 +1056,13 @@ def _reconcile_skills(self, command_names: List[str]) -> None: ) init_opts = load_init_options(self.project_root) selected_ai = init_opts.get("ai") if isinstance(init_opts, dict) else "" + integration_parsed_options = ( + init_opts.get("integration_parsed_options") + if isinstance(init_opts, dict) + else None + ) + if not isinstance(integration_parsed_options, dict): + integration_parsed_options = None if isinstance(selected_ai, str): body = registrar.resolve_skill_placeholders( selected_ai, fm, body, self.project_root @@ -1078,7 +1085,10 @@ def _reconcile_skills(self, command_names: List[str]) -> None: from .integrations import get_integration integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None if integration is not None and hasattr(integration, "post_process_skill_content"): - skill_content = integration.post_process_skill_content(skill_content) + skill_content = integration.post_process_skill_content( + skill_content, + parsed_options=integration_parsed_options, + ) skill_file.write_text(skill_content, encoding="utf-8") except Exception: pass # best-effort override skill restoration @@ -1263,6 +1273,9 @@ def _register_skills( selected_ai = init_opts.get("ai") if not isinstance(selected_ai, str): return [] + integration_parsed_options = init_opts.get("integration_parsed_options") + if not isinstance(integration_parsed_options, dict): + integration_parsed_options = None ai_skills_enabled = is_ai_skills_enabled(init_opts) registrar = CommandRegistrar() integration = get_integration(selected_ai) @@ -1355,7 +1368,8 @@ def _register_skills( ) if integration is not None and hasattr(integration, "post_process_skill_content"): skill_content = integration.post_process_skill_content( - skill_content + skill_content, + parsed_options=integration_parsed_options, ) skill_file = skill_subdir / "SKILL.md" @@ -1394,6 +1408,9 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: selected_ai = init_opts.get("ai") registrar = CommandRegistrar() integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None + integration_parsed_options = init_opts.get("integration_parsed_options") + if not isinstance(integration_parsed_options, dict): + integration_parsed_options = None extension_restore_index = self._build_extension_skill_restore_index() for skill_name in skill_names: @@ -1452,7 +1469,8 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: ) if integration is not None and hasattr(integration, "post_process_skill_content"): skill_content = integration.post_process_skill_content( - skill_content + skill_content, + parsed_options=integration_parsed_options, ) skill_file.write_text(skill_content, encoding="utf-8") continue @@ -1488,7 +1506,8 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: ) if integration is not None and hasattr(integration, "post_process_skill_content"): skill_content = integration.post_process_skill_content( - skill_content + skill_content, + parsed_options=integration_parsed_options, ) skill_file.write_text(skill_content, encoding="utf-8") else: diff --git a/tests/integrations/test_integration_claude.py b/tests/integrations/test_integration_claude.py index e1bda1fb68..e46b4e3857 100644 --- a/tests/integrations/test_integration_claude.py +++ b/tests/integrations/test_integration_claude.py @@ -38,6 +38,13 @@ def test_context_file(self): integration = get_integration("claude") assert integration.context_file == "CLAUDE.md" + def test_options_include_no_model_invocation(self): + integration = get_integration("claude") + options = {option.name: option for option in integration.options()} + assert "--no-model-invocation" in options + assert options["--no-model-invocation"].is_flag is True + assert options["--no-model-invocation"].default is False + def test_setup_creates_skill_files(self, tmp_path): integration = get_integration("claude") manifest = IntegrationManifest("claude", tmp_path) @@ -448,7 +455,7 @@ def test_inject_argument_hint_skips_if_already_present(self): class TestClaudeDisableModelInvocation: - """Verify disable-model-invocation is false for Claude skills.""" + """Verify Claude skill model-invocation frontmatter.""" def test_setup_sets_disable_model_invocation_false(self, tmp_path): """Generated SKILL.md files must have disable-model-invocation: false.""" @@ -465,6 +472,151 @@ def test_setup_sets_disable_model_invocation_false(self, tmp_path): f"{f.parent.name}: expected disable-model-invocation: false" ) + def test_setup_honors_no_model_invocation_option(self, tmp_path): + """--no-model-invocation sets generated Claude skills to user-invocable only.""" + i = get_integration("claude") + m = IntegrationManifest("claude", tmp_path) + created = i.setup( + tmp_path, + m, + parsed_options={"no_model_invocation": True}, + script_type="sh", + ) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) > 0 + for f in skill_files: + content = f.read_text(encoding="utf-8") + parts = content.split("---", 2) + parsed = yaml.safe_load(parts[1]) + assert parsed["user-invocable"] is True + assert parsed["disable-model-invocation"] is True, ( + f"{f.parent.name}: expected disable-model-invocation: true" + ) + + def test_init_no_model_invocation_persists_and_updates_claude_skills( + self, + tmp_path, + ): + """init stores the option and applies it to core and extension skills.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / "claude-no-model" + project.mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + runner = CliRunner() + result = runner.invoke( + app, + [ + "init", + "--here", + "--integration", + "claude", + "--integration-options", + "--no-model-invocation", + "--script", + "sh", + "--no-git", + "--ignore-agent-tools", + ], + catch_exceptions=False, + ) + finally: + os.chdir(old_cwd) + + assert result.exit_code == 0, result.output + + init_options = json.loads( + (project / ".specify" / "init-options.json").read_text(encoding="utf-8") + ) + assert init_options["integration_options"] == "--no-model-invocation" + assert init_options["integration_parsed_options"] == { + "no_model_invocation": True, + } + + integration_state = json.loads( + (project / ".specify" / "integration.json").read_text(encoding="utf-8") + ) + claude_settings = integration_state["integration_settings"]["claude"] + assert claude_settings["raw_options"] == "--no-model-invocation" + assert claude_settings["parsed_options"] == {"no_model_invocation": True} + + skill_files = [ + project / ".claude" / "skills" / "speckit-plan" / "SKILL.md", + project + / ".claude" + / "skills" + / "speckit-agent-context-update" + / "SKILL.md", + ] + for skill_file in skill_files: + assert skill_file.exists() + parts = skill_file.read_text(encoding="utf-8").split("---", 2) + parsed = yaml.safe_load(parts[1]) + assert parsed["disable-model-invocation"] is True + + def test_upgrade_reuses_stored_no_model_invocation_option(self, tmp_path): + """upgrade without --integration-options preserves the stored opt-out.""" + from typer.testing import CliRunner + from specify_cli import app + + project = tmp_path / "claude-upgrade-no-model" + project.mkdir() + runner = CliRunner() + old_cwd = os.getcwd() + try: + os.chdir(project) + init_result = runner.invoke( + app, + [ + "init", + "--here", + "--integration", + "claude", + "--integration-options", + "--no-model-invocation", + "--script", + "sh", + "--no-git", + "--ignore-agent-tools", + ], + catch_exceptions=False, + ) + assert init_result.exit_code == 0, init_result.output + + plan_skill = ( + project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + ) + plan_skill.write_text( + plan_skill.read_text(encoding="utf-8").replace( + "disable-model-invocation: true", + "disable-model-invocation: false", + ), + encoding="utf-8", + ) + + upgrade_result = runner.invoke( + app, + ["integration", "upgrade", "claude", "--force"], + catch_exceptions=False, + ) + finally: + os.chdir(old_cwd) + + assert upgrade_result.exit_code == 0, upgrade_result.output + parts = plan_skill.read_text(encoding="utf-8").split("---", 2) + parsed = yaml.safe_load(parts[1]) + assert parsed["disable-model-invocation"] is True + + init_options = json.loads( + (project / ".specify" / "init-options.json").read_text(encoding="utf-8") + ) + assert init_options["integration_parsed_options"] == { + "no_model_invocation": True, + } + def test_disable_model_invocation_not_true(self, tmp_path): """No Claude skill should have disable-model-invocation: true.""" i = get_integration("claude") @@ -579,6 +731,17 @@ def test_post_process_injects_all_claude_flags(self): assert "disable-model-invocation: false" in result assert "replace dots" in result + def test_post_process_honors_no_model_invocation_option(self): + """post_process_skill_content should honor persisted parsed options.""" + i = get_integration("claude") + content = "---\nname: test\ndescription: test\n---\n\nBody\n" + result = i.post_process_skill_content( + content, + parsed_options={"no_model_invocation": True}, + ) + assert "user-invocable: true" in result + assert "disable-model-invocation: true" in result + class TestSpeckitManifestRecordsSkippedFiles: """Regression test for issue #2107. diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index 22eb63dfd8..163d0b1552 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -303,6 +303,28 @@ def test_skill_md_has_parseable_yaml(self, skills_project, extension_dir): assert "description" in parsed assert parsed["disable-model-invocation"] is False + def test_claude_skill_respects_no_model_invocation_init_option( + self, + skills_project, + extension_dir, + ): + """Extension skills inherit Claude's persisted no-model-invocation option.""" + project_dir, skills_dir = skills_project + opts_file = project_dir / ".specify" / "init-options.json" + opts = json.loads(opts_file.read_text(encoding="utf-8")) + opts["integration_options"] = "--no-model-invocation" + opts["integration_parsed_options"] = {"no_model_invocation": True} + opts_file.write_text(json.dumps(opts), encoding="utf-8") + + manager = ExtensionManager(project_dir) + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md" + parsed = yaml.safe_load(skill_file.read_text().split("---", 2)[1]) + assert parsed["disable-model-invocation"] is True + def test_no_skills_when_ai_skills_disabled(self, no_skills_project, extension_dir): """No skills should be created when ai_skills is false.""" manager = ExtensionManager(no_skills_project)