diff --git a/AGENTS.md b/AGENTS.md index 82fde69548..9c1ce688a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -260,13 +260,13 @@ The base classes handle most work automatically. Override only when the agent de | Override | When to use | Example | |---|---|---| | `command_filename(template_name)` | Custom file naming or extension | Copilot → `speckit.{name}.agent.md` | -| `options()` | Integration-specific CLI flags via `--integration-options` | Codex → `--skills` flag, Copilot → `--skills` flag | -| `setup()` | Custom install logic (companion files, settings merge) | Copilot → `.agent.md` + `.prompt.md` + `.vscode/settings.json` (default) or `speckit-/SKILL.md` (skills mode) | +| `options()` | Integration-specific CLI flags via `--integration-options` | Codex → `--skills` flag, Copilot → `--commands` flag | +| `setup()` | Custom install logic (companion files, settings merge) | Copilot → `speckit-/SKILL.md` (default) or `.agent.md` + `.prompt.md` + `.vscode/settings.json` (`--commands`) | | `teardown()` | Custom uninstall logic | Rarely needed; base handles manifest-tracked files | **Example — Copilot (fully custom `setup`):** -Copilot extends `IntegrationBase` directly because it creates `.agent.md` commands, companion `.prompt.md` files, and merges `.vscode/settings.json`. It also supports a `--skills` mode that scaffolds `speckit-/SKILL.md` under `.github/skills/` using composition with an internal `_CopilotSkillsHelper`. See `src/specify_cli/integrations/copilot/__init__.py` for the full implementation. +Copilot extends `IntegrationBase` directly because it supports two layouts. It scaffolds `speckit-/SKILL.md` under `.github/skills/` by default using composition with an internal `_CopilotSkillsHelper`. Its `--commands` mode creates `.agent.md` commands, companion `.prompt.md` files, and merges `.vscode/settings.json`. See `src/specify_cli/integrations/copilot/__init__.py` for the full implementation. ### 7. Update Devcontainer files (Optional) @@ -415,36 +415,28 @@ Some agents require custom processing beyond the standard template transformatio ### Copilot Integration -GitHub Copilot has unique requirements: +GitHub Copilot uses skills by default, scaffolded as +`speckit-/SKILL.md` under `.github/skills/`. -- Commands use `.agent.md` extension (not `.md`) -- Each command gets a companion `.prompt.md` file in `.github/prompts/` -- Installs `.vscode/settings.json` with prompt file recommendations -- Context file lives at `.github/copilot-instructions.md` - -Implementation: Extends `IntegrationBase` with custom `setup()` method that: +**Commands mode (`--commands`):** Copilot also supports a commands-based layout +via `--integration-options="--commands"`. When enabled: -1. Processes templates with `process_template()` -2. Generates companion `.prompt.md` files -3. Merges VS Code settings - -**Skills mode (`--skills`):** Copilot also supports an alternative skills-based layout -via `--integration-options="--skills"`. When enabled: +- Commands use `.agent.md` extension under `.github/agents/` +- Each command gets a companion `.prompt.md` file in `.github/prompts/` +- `.vscode/settings.json` is merged with prompt file recommendations +- `build_command_invocation()` returns bare args for `--agent` dispatch -- Commands are scaffolded as `speckit-/SKILL.md` under `.github/skills/` -- No companion `.prompt.md` files are generated -- No `.vscode/settings.json` merge -- `post_process_skill_content()` injects a `mode: speckit.` frontmatter field -- `build_command_invocation()` returns `/speckit-` instead of bare args +In the default skills mode, no companion prompts or VS Code settings merge are +created, and `build_command_invocation()` returns `/speckit-`. The two modes are mutually exclusive — a project uses one or the other: ```bash -# Default mode: .agent.md agents + .prompt.md companions + settings merge +# Default skills mode: speckit-/SKILL.md under .github/skills/ specify init my-project --integration copilot -# Skills mode: speckit-/SKILL.md under .github/skills/ -specify init my-project --integration copilot --integration-options="--skills" +# Commands mode: .agent.md agents + .prompt.md companions + settings merge +specify init my-project --integration copilot --integration-options="--commands" ``` ### Forge Integration diff --git a/docs/reference/integrations.md b/docs/reference/integrations.md index a12337316b..808d0cf752 100644 --- a/docs/reference/integrations.md +++ b/docs/reference/integrations.md @@ -20,7 +20,7 @@ The Specify CLI supports a wide range of AI coding agents. When you run `specify | [Firebender](https://firebender.com/) | `firebender` | IDE-based agent for Android Studio / IntelliJ | | [Forge](https://forgecode.dev/) | `forge` | | | [Gemini CLI](https://github.com/google-gemini/gemini-cli) | `gemini` | | -| [GitHub Copilot](https://code.visualstudio.com/) | `copilot` | Defaults to legacy markdown mode: `.agent.md` command files under `.github/agents/`, companion `.prompt.md` files under `.github/prompts/`, and a `.vscode/settings.json` merge. Pass `--integration-options="--skills"` to scaffold skills as `speckit-/SKILL.md` under `.github/skills/` instead. Legacy markdown mode is deprecated and will stop being the default in a future release. | +| [GitHub Copilot](https://code.visualstudio.com/) | `copilot` | Skills-based by default; installs `speckit-/SKILL.md` under `.github/skills/`. Pass `--integration-options="--commands"` to use the supported commands layout: `.agent.md` files under `.github/agents/`, companion `.prompt.md` files under `.github/prompts/`, and a `.vscode/settings.json` merge. | | [Goose](https://goose-docs.ai/) | `goose` | Uses YAML recipe format in `.goose/recipes/` | | [Grok Build](https://docs.x.ai/build/overview) | `grok` | Skills-based integration; installs skills into `.grok/skills` and invokes them as `/speckit-` | | [Hermes](https://github.com/NousResearch/hermes-agent) | `hermes` | Skills-based integration; installs skills globally into `~/.hermes/skills/` | @@ -234,7 +234,8 @@ Some integrations accept additional options via `--integration-options`: | ----------- | ------------------- | -------------------------------------------------------------- | | `generic` | `--commands-dir` | Required. Directory for command files | | `kimi` | `--migrate-legacy` | Migrate legacy `.kimi/skills/` installs to `.kimi-code/skills/` (including dotted→hyphenated skill naming, e.g. `speckit.xxx` → `speckit-xxx`) | -| `copilot` | `--skills` | Scaffold commands as agent skills (`speckit-/SKILL.md` under `.github/skills/`, invoked as `/speckit-`) instead of the default legacy markdown mode (`.github/agents/*.agent.md` plus `.github/prompts/*.prompt.md` and a `.vscode/settings.json` merge). Without this flag, install warns that legacy markdown mode is deprecated. | +| `copilot` | `--commands` | Scaffold `.github/agents/*.agent.md` commands with `.github/prompts/*.prompt.md` companions and merge `.vscode/settings.json` instead of using the default skills layout. | +| `copilot` | `--skills` | Force the default skills layout, overriding an existing commands layout during an explicit migration. | Example: diff --git a/src/specify_cli/integration_runtime.py b/src/specify_cli/integration_runtime.py index eef44574cb..efcd8a9e63 100644 --- a/src/specify_cli/integration_runtime.py +++ b/src/specify_cli/integration_runtime.py @@ -70,8 +70,8 @@ def with_integration_setting( # ``script_type`` changes (``parsed_options`` and ``raw_options`` both # None), the previously-stored ``parsed_options`` are retained above, so # deriving the separator from the argument (None) would drop an - # options-dependent separator (e.g. Copilot ``--skills`` -> "-") back to - # the default ".". + # options-dependent separator (e.g. Copilot ``--commands`` -> ".") back to + # the default "-". current["invoke_separator"] = integration.effective_invoke_separator( current.get("parsed_options"), project_root ) diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index 6f0a51b81c..2e71c26e94 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -331,6 +331,14 @@ def integration_switch( selected_script = _resolve_script_type(project_root, script) + # Resolve and validate target options before uninstalling the current + # integration. Invalid options must not leave the project partially + # switched with the previous integration already removed. + target_raw_options, target_parsed_options = _resolve_integration_options( + target_integration, current, target, integration_options + ) + target_integration.is_skills_mode(target_parsed_options, project_root) + # Phase 1: Uninstall current integration (if any) if installed_key: current_integration = get_integration(installed_key) @@ -403,7 +411,10 @@ def integration_switch( fallback_key = installed_keys[0] fallback_integration = get_integration(fallback_key) if fallback_integration is not None: - raw_options, parsed_options = _resolve_integration_options( + ( + fallback_raw_options, + fallback_parsed_options, + ) = _resolve_integration_options( fallback_integration, current, fallback_key, None ) _set_default_integration_or_exit( @@ -412,8 +423,8 @@ def integration_switch( fallback_key, fallback_integration, installed_keys, - raw_options=raw_options, - parsed_options=parsed_options, + raw_options=fallback_raw_options, + parsed_options=fallback_parsed_options, ) else: _write_integration_json( @@ -423,13 +434,6 @@ def integration_switch( _remove_integration_json(project_root) current = _read_integration_json(project_root) - # Build parsed options from --integration-options so the integration - # can determine its effective invoke separator before shared infra - # is installed. - raw_options, parsed_options = _resolve_integration_options( - target_integration, current, target, integration_options - ) - # Refresh shared infrastructure to the current CLI version. Switching # integrations is exactly when stale vendored shared scripts (e.g. # update-agent-context.sh that pre-dates the target integration's @@ -445,11 +449,11 @@ def integration_switch( force=refresh_shared_infra, refresh_managed=True, invoke_separator=_invoke_separator_for_integration( - target_integration, current, target, parsed_options, + target_integration, current, target, target_parsed_options, project_root=project_root, ), invoke_prefix=_invoke_prefix_for_integration( - target_integration, target, parsed_options, project_root + target_integration, target, target_parsed_options, project_root ), refresh_hint=( "To overwrite customizations, re-run with " @@ -471,14 +475,14 @@ def integration_switch( target_integration.key, target_integration.config, project_root, - parsed_options, + target_parsed_options, ) try: target_integration.setup( project_root, manifest, - parsed_options=parsed_options, + parsed_options=target_parsed_options, script_type=selected_script, - raw_options=raw_options, + raw_options=target_raw_options, events=events_map, ) manifest.save() @@ -489,8 +493,8 @@ def integration_switch( target_integration, _dedupe_integration_keys([*installed_keys, target_integration.key]), script_type=selected_script, - raw_options=raw_options, - parsed_options=parsed_options, + raw_options=target_raw_options, + parsed_options=target_parsed_options, ) except Exception as exc: diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index cca4f13976..ebcf8dde12 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -225,8 +225,8 @@ def is_skills_mode( on-disk layout to avoid silently migrating an existing project to a different mode. The default ignores it. - The default (command-first integrations, e.g. Copilot's default - layout) is skills mode only when ``--skills`` was requested. + The default for command-first integrations is skills mode only when + ``--skills`` was requested. ``SkillsIntegration`` overrides this to return ``True`` by default; skills-first integrations that expose a legacy opt-out (e.g. Bob) override it to honor their own flag. diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index e6f86e8991..9c6b33b2a5 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -1,13 +1,19 @@ """Copilot integration — GitHub Copilot in VS Code. -Copilot has several unique behaviors compared to standard markdown agents: +Copilot supports two layouts: +- Skills are the default and use ``speckit-/SKILL.md`` directories under + ``.github/skills/`` +- ``--commands`` uses ``.agent.md`` files, companion ``.prompt.md`` files, and + a VS Code settings merge + +The two modes are mutually exclusive. The commands layout remains supported, +but is no longer the preferred default. + +The commands layout has several unique behaviors compared to standard markdown +agents: - Commands use ``.agent.md`` extension (not ``.md``) - Each command gets a companion ``.prompt.md`` file in ``.github/prompts/`` - Installs ``.vscode/settings.json`` with prompt file recommendations - -When ``--skills`` is passed via ``--integration-options``, Copilot scaffolds -commands as ``speckit-/SKILL.md`` directories under ``.github/skills/`` -instead. The two modes are mutually exclusive. """ from __future__ import annotations @@ -19,9 +25,24 @@ from pathlib import Path from typing import Any +import typer + from ..base import IntegrationBase, IntegrationOption, SkillsIntegration from ..manifest import IntegrationManifest +_COPILOT_CORE_COMMANDS = { + "analyze", + "checklist", + "clarify", + "constitution", + "converge", + "implement", + "plan", + "specify", + "tasks", + "taskstoissues", +} + def _copilot_executable() -> str: """Return the executable name for Copilot CLI on this platform. @@ -57,22 +78,24 @@ def _allow_all() -> bool: return True -def _warn_legacy_markdown_default() -> None: - """Warn that Copilot's default markdown scaffold is being phased out.""" - warnings.warn( - "Copilot legacy markdown mode is deprecated and will stop being the " - 'default in a future Spec Kit release; pass --integration-options "--skills" ' - "to opt in to Copilot skills mode now.", - UserWarning, - stacklevel=3, - ) +def _validate_mode_options(parsed_options: dict[str, Any] | None) -> None: + """Reject the two explicit Copilot layout selectors used together.""" + opts = parsed_options or {} + if opts.get("skills") and opts.get("commands"): + from ..._console import console + + console.print( + "[red]Error:[/red] --skills and --commands are mutually exclusive; " + "pass only one." + ) + raise typer.Exit(1) class _CopilotSkillsHelper(SkillsIntegration): """Internal helper used when Copilot is scaffolded in skills mode. - Not registered in the integration registry — only used as a delegate - by ``CopilotIntegration`` when ``--skills`` is passed. + Not registered in the integration registry — only used as the default + skills-layout delegate by ``CopilotIntegration``. """ key = "copilot" @@ -94,13 +117,11 @@ class _CopilotSkillsHelper(SkillsIntegration): class CopilotIntegration(IntegrationBase): """Integration for GitHub Copilot (VS Code IDE + CLI). - The IDE integration (``requires_cli: False``) installs ``.agent.md`` - command files. Workflow dispatch additionally requires the - ``copilot`` CLI to be installed separately. - - When ``--skills`` is passed via ``--integration-options``, commands - are scaffolded as ``speckit-/SKILL.md`` under ``.github/skills/`` - instead of the default ``.agent.md`` + ``.prompt.md`` layout. + The default IDE integration (``requires_cli: False``) installs skills under + ``.github/skills/``. Pass ``--commands`` via ``--integration-options`` to + install the supported ``.agent.md`` + ``.prompt.md`` layout instead. + Workflow dispatch additionally requires the ``copilot`` CLI to be installed + separately. """ key = "copilot" @@ -117,6 +138,7 @@ class CopilotIntegration(IntegrationBase): "args": "$ARGUMENTS", "extension": ".agent.md", } + invoke_separator = "-" CANONICAL_TO_NATIVE = { "session_start": "sessionStart", @@ -132,38 +154,91 @@ class CopilotIntegration(IntegrationBase): events_format = "copilot-json" # Mutable flag set by setup() — indicates the active scaffolding mode. - _skills_mode: bool = False + _skills_mode: bool = True def effective_invoke_separator( self, parsed_options: dict[str, Any] | None = None, project_root: Path | None = None, ) -> str: - """Return ``"-"`` when skills mode is requested, ``"."`` otherwise.""" - if parsed_options and parsed_options.get("skills"): - return "-" - if self._skills_mode: - return "-" - return self.invoke_separator + """Return the separator for the resolved Copilot layout.""" + return "-" if self.is_skills_mode(parsed_options, project_root) else "." def is_skills_mode( self, parsed_options: dict[str, Any] | None = None, project_root: Path | None = None, ) -> bool: - """Copilot is skills mode when ``--skills`` was requested. + """Copilot defaults to skills; ``--commands`` opts into commands mode. - On the init path ``setup()`` has already recorded the choice in - ``self._skills_mode``; on the ``use``/``install`` path (where no - ``setup()`` runs) the signal comes from *parsed_options* (#3550), which - round-trips because ``--skills`` is persisted in the stored options. + Explicit flags override on-disk detection. Without a flag, existing + projects retain their managed Spec Kit layout while fresh projects use + skills. This prevents ``use`` and ``upgrade`` from silently migrating + projects created before skills became the default. """ - if parsed_options and parsed_options.get("skills"): + opts = parsed_options or {} + _validate_mode_options(opts) + if opts.get("skills"): return True - return self._skills_mode + if opts.get("commands"): + return False + if project_root is not None: + project_root = Path(project_root) + manifest_path = ( + project_root + / ".specify" + / "integrations" + / "copilot.manifest.json" + ) + if manifest_path.is_file(): + try: + manifest_files = IntegrationManifest.load( + self.key, Path(project_root) + ).files + except (OSError, ValueError): + manifest_files = None + if manifest_files is not None and any( + path.startswith(".github/skills/speckit-") + and path.endswith("/SKILL.md") + for path in manifest_files + ): + return True + if manifest_files is not None and any( + path.startswith(".github/agents/speckit.") + and path.endswith(".agent.md") + for path in manifest_files + ): + return False + + github_dir = project_root / ".github" + has_managed_skills = any( + ( + github_dir + / "skills" + / f"speckit-{command}" + / "SKILL.md" + ).is_file() + for command in _COPILOT_CORE_COMMANDS + ) + has_managed_commands = any( + ( + github_dir + / "agents" + / f"speckit.{command}.agent.md" + ).is_file() + or ( + github_dir + / "prompts" + / f"speckit.{command}.prompt.md" + ).is_file() + for command in _COPILOT_CORE_COMMANDS + ) + if has_managed_commands and not has_managed_skills: + return False + return True def invoke_separator_for_mode(self, skills_enabled: bool) -> str: - """Skills projects render ``/speckit-``; default markdown ``.``. + """Skills projects render ``/speckit-``; commands use ``.``. Copilot is dual-layout, so — like Bob — the command-reference separator depends on the persisted ``ai_skills`` state rather than a @@ -171,7 +246,7 @@ def invoke_separator_for_mode(self, skills_enabled: bool) -> str: Copilot skills project consistent with ``build_command_invocation`` (which emits ``/speckit-``). """ - return "-" if skills_enabled else self.invoke_separator + return "-" if skills_enabled else "." @classmethod def options(cls) -> list[IntegrationOption]: @@ -184,7 +259,22 @@ def options(cls) -> list[IntegrationOption]: "--skills", is_flag=True, default=False, - help="Scaffold commands as agent skills (speckit-/SKILL.md) instead of .agent.md files", + help=( + "Force the default skills layout (.github/skills/), " + "overriding on-disk auto-detection" + ), + ), + ) + opts.append( + IntegrationOption( + "--commands", + is_flag=True, + default=False, + help=( + "Scaffold .github/agents/*.agent.md commands with companion " + ".github/prompts/*.prompt.md files instead of the default " + "skills layout" + ), ), ) return opts @@ -228,8 +318,8 @@ def build_exec_args( def build_command_invocation(self, command_name: str, args: str = "") -> str: """Build the native invocation for a Copilot command. - Default mode: agents are not slash-commands — return args as prompt. - Skills mode: ``/speckit-`` slash-command dispatch. + Commands mode: agents are not slash-commands — return args as prompt. + Skills mode (default): ``/speckit-`` slash-command dispatch. """ if self._skills_mode: stem = command_name @@ -266,15 +356,11 @@ def dispatch_command( if stem.startswith("speckit."): stem = stem[len("speckit."):] - # Detect skills mode from project layout when not set via setup() - skills_mode = self._skills_mode - if not skills_mode and project_root: - skills_dir = project_root / ".github" / "skills" - if skills_dir.is_dir(): - skills_mode = any( - d.is_dir() and (d / "SKILL.md").is_file() - for d in skills_dir.glob("speckit-*") - ) + skills_mode = ( + self.is_skills_mode(project_root=project_root) + if project_root + else self._skills_mode + ) if skills_mode: prompt = "/speckit-" + stem.replace(".", "-") @@ -366,20 +452,18 @@ def setup( parsed_options: dict[str, Any] | None = None, **opts: Any, ) -> list[Path]: - """Install copilot commands, companion prompts, and VS Code settings. + """Install Copilot skills or the opt-in commands layout. - When ``parsed_options["skills"]`` is truthy, delegates to skills - scaffolding (``speckit-/SKILL.md`` under ``.github/skills/``). - Otherwise uses the default ``.agent.md`` + ``.prompt.md`` layout. + Skills are the default. ``parsed_options["commands"]`` selects + ``.agent.md`` files, companion prompts, and the VS Code settings merge. + Existing managed command layouts are preserved when no mode is explicit. """ parsed_options = parsed_options or {} - self._skills_mode = bool(parsed_options.get("skills")) + self._skills_mode = self.is_skills_mode(parsed_options, project_root) if self._skills_mode: created = self._setup_skills(project_root, manifest, parsed_options, **opts) else: - if "skills" not in parsed_options: - _warn_legacy_markdown_default() - created = self._setup_default(project_root, manifest, parsed_options, **opts) + created = self._setup_commands(project_root, manifest, parsed_options, **opts) # Install agent runtime events event_files = self.emit_events( @@ -388,14 +472,14 @@ def setup( created.extend(event_files) return created - def _setup_default( + def _setup_commands( self, project_root: Path, manifest: IntegrationManifest, parsed_options: dict[str, Any] | None = None, **opts: Any, ) -> list[Path]: - """Default mode: .agent.md + .prompt.md + VS Code settings merge.""" + """Commands mode: .agent.md + .prompt.md + VS Code settings merge.""" project_root_resolved = project_root.resolve() if manifest.project_root != project_root_resolved: raise ValueError( diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 01d35c027f..97ad30d81a 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -69,8 +69,11 @@ def test_integration_copilot_creates_files(self, tmp_path): finally: os.chdir(old_cwd) assert result.exit_code == 0, f"init failed: {result.output}" - assert (project / ".github" / "agents" / "speckit.plan.agent.md").exists() - assert (project / ".github" / "prompts" / "speckit.plan.prompt.md").exists() + assert ( + project / ".github" / "skills" / "speckit-plan" / "SKILL.md" + ).exists() + assert not (project / ".github" / "agents").exists() + assert not (project / ".github" / "prompts").exists() assert (project / ".specify" / "scripts" / "bash" / "common.sh").exists() data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) @@ -78,6 +81,7 @@ def test_integration_copilot_creates_files(self, tmp_path): opts = json.loads((project / ".specify" / "init-options.json").read_text(encoding="utf-8")) assert opts["integration"] == "copilot" + assert opts["ai_skills"] is True # init must not leave any legacy agent-context keys in init-options.json assert "context_file" not in opts @@ -111,7 +115,9 @@ def fail_select(*_args, **_kwargs): assert result.exit_code == 0, result.output assert f"defaulting to '{specify_cli.DEFAULT_INIT_INTEGRATION}'" in result.output - assert (project / ".github" / "agents" / "speckit.plan.agent.md").exists() + assert ( + project / ".github" / "skills" / "speckit-plan" / "SKILL.md" + ).exists() data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) assert data["integration"] == specify_cli.DEFAULT_INIT_INTEGRATION @@ -188,7 +194,9 @@ def test_integration_copilot_auto_promotes(self, tmp_path): finally: os.chdir(old_cwd) assert result.exit_code == 0 - assert (project / ".github" / "agents" / "speckit.plan.agent.md").exists() + assert ( + project / ".github" / "skills" / "speckit-plan" / "SKILL.md" + ).exists() def test_init_optional_preset_failure_reports_target_and_continues( self, tmp_path, monkeypatch @@ -1311,7 +1319,7 @@ def test_full_init_claude_resolves_page_templates(self, tmp_path): assert "/speckit.specify" not in script_content def test_full_init_copilot_resolves_page_templates(self, tmp_path): - """Full CLI init with Copilot (markdown agent) produces dot refs in page templates.""" + """Default Copilot skills mode produces hyphen refs in page templates.""" from typer.testing import CliRunner from specify_cli import app @@ -1333,27 +1341,28 @@ def test_full_init_copilot_resolves_page_templates(self, tmp_path): plan = project / ".specify" / "templates" / "plan-template.md" content = plan.read_text(encoding="utf-8") - assert "/speckit.plan" in content, "Copilot (markdown) should use /speckit.plan" + assert "/speckit-plan" in content, "Copilot skills should use /speckit-plan" + assert "/speckit.plan" not in content assert "__SPECKIT_COMMAND_" not in content script_content = self._combined_script_content(project, "sh") - assert "/speckit.specify" in script_content - assert "/speckit-specify" not in script_content + assert "/speckit-specify" in script_content + assert "/speckit.specify" not in script_content - def test_full_init_copilot_skills_resolves_page_templates(self, tmp_path): - """Full CLI init with Copilot --skills produces hyphen refs in page templates.""" + def test_full_init_copilot_commands_resolves_page_templates(self, tmp_path): + """Copilot --commands produces dot refs in page templates.""" from typer.testing import CliRunner from specify_cli import app runner = CliRunner() - project = tmp_path / "init-copilot-skills" + project = tmp_path / "init-copilot-commands" old_cwd = os.getcwd() try: os.chdir(tmp_path) result = runner.invoke(app, [ "init", str(project), "--integration", "copilot", - "--integration-options", "--skills", + "--integration-options", "--commands", "--script", "sh", "--ignore-agent-tools", ], catch_exceptions=False) @@ -1364,13 +1373,13 @@ def test_full_init_copilot_skills_resolves_page_templates(self, tmp_path): plan = project / ".specify" / "templates" / "plan-template.md" content = plan.read_text(encoding="utf-8") - assert "/speckit-plan" in content, "Copilot --skills should use /speckit-plan" - assert "/speckit.plan" not in content, "dot-notation leaked into Copilot skills page template" + assert "/speckit.plan" in content, "Copilot --commands should use /speckit.plan" + assert "/speckit-plan" not in content assert "__SPECKIT_COMMAND_" not in content script_content = self._combined_script_content(project, "sh") - assert "/speckit-specify" in script_content - assert "/speckit.specify" not in script_content + assert "/speckit.specify" in script_content + assert "/speckit-specify" not in script_content class TestIntegrationCatalogDiscoveryCLI: diff --git a/tests/integrations/test_extra_args.py b/tests/integrations/test_extra_args.py index e329c88801..84f48a5fd0 100644 --- a/tests/integrations/test_extra_args.py +++ b/tests/integrations/test_extra_args.py @@ -426,7 +426,7 @@ class _Result: return _Result() -def test_copilot_dispatch_command_includes_extra_args(monkeypatch): +def test_copilot_commands_dispatch_includes_extra_args(monkeypatch): """Locks the bypass fix: `CopilotIntegration.dispatch_command` must honour `SPECKIT_INTEGRATION_COPILOT_EXTRA_ARGS`, not just `build_exec_args`. """ @@ -441,9 +441,9 @@ def test_copilot_dispatch_command_includes_extra_args(monkeypatch): "SPECKIT_INTEGRATION_COPILOT_EXTRA_ARGS", "--allow-tool 'shell(echo)'" ) - CopilotIntegration().dispatch_command( - "speckit.plan", args="body", stream=False - ) + integration = CopilotIntegration() + integration._skills_mode = False + integration.dispatch_command("speckit.plan", args="body", stream=False) assert capture.captured_args is not None # Hook inserted between `-p prompt` and the canonical Copilot flags. diff --git a/tests/integrations/test_integration_copilot.py b/tests/integrations/test_integration_copilot.py index 6474250976..7a680b7dd4 100644 --- a/tests/integrations/test_integration_copilot.py +++ b/tests/integrations/test_integration_copilot.py @@ -2,16 +2,16 @@ import json import os -import warnings import pytest +import typer import yaml from specify_cli.integrations import get_integration from specify_cli.integrations.manifest import IntegrationManifest -class TestCopilotIntegration: +class TestCopilotCommandsMode: def test_copilot_key_and_config(self): copilot = get_integration("copilot") assert copilot is not None @@ -28,7 +28,7 @@ def test_setup_creates_agent_md_files(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m) + created = copilot.setup(tmp_path, m, parsed_options={"commands": True}) assert len(created) > 0 agent_files = [f for f in created if ".agent." in f.name] assert len(agent_files) > 0 @@ -36,36 +36,11 @@ def test_setup_creates_agent_md_files(self, tmp_path): assert f.parent == tmp_path / ".github" / "agents" assert f.name.endswith(".agent.md") - def test_setup_warns_legacy_markdown_default_is_deprecated(self, tmp_path): - from specify_cli.integrations.copilot import CopilotIntegration - copilot = CopilotIntegration() - m = IntegrationManifest("copilot", tmp_path) - - with pytest.warns(UserWarning, match="Copilot legacy markdown mode is deprecated"): - created = copilot.setup(tmp_path, m) - - assert any(f.name.endswith(".agent.md") for f in created) - - def test_skills_setup_does_not_warn_about_legacy_default(self, tmp_path): - from specify_cli.integrations.copilot import CopilotIntegration - copilot = CopilotIntegration() - m = IntegrationManifest("copilot", tmp_path) - - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") - created = copilot.setup(tmp_path, m, parsed_options={"skills": True}) - - assert not any( - "Copilot legacy markdown mode is deprecated" in str(item.message) - for item in caught - ) - assert any(f.name == "SKILL.md" for f in created) - def test_setup_creates_companion_prompts(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m) + created = copilot.setup(tmp_path, m, parsed_options={"commands": True}) prompt_files = [f for f in created if f.parent.name == "prompts"] assert len(prompt_files) > 0 for f in prompt_files: @@ -77,7 +52,7 @@ def test_agent_and_prompt_counts_match(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m) + created = copilot.setup(tmp_path, m, parsed_options={"commands": True}) agents = [f for f in created if ".agent.md" in f.name] prompts = [f for f in created if ".prompt.md" in f.name] assert len(agents) == len(prompts) @@ -87,7 +62,7 @@ def test_setup_creates_vscode_settings_new(self, tmp_path): copilot = CopilotIntegration() assert copilot._vscode_settings_path() is not None m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m) + created = copilot.setup(tmp_path, m, parsed_options={"commands": True}) settings = tmp_path / ".vscode" / "settings.json" assert settings.exists() assert settings in created @@ -101,7 +76,7 @@ def test_setup_merges_existing_vscode_settings(self, tmp_path): existing = {"editor.fontSize": 14, "custom.setting": True} (vscode_dir / "settings.json").write_text(json.dumps(existing, indent=4), encoding="utf-8") m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m) + created = copilot.setup(tmp_path, m, parsed_options={"commands": True}) settings = tmp_path / ".vscode" / "settings.json" data = json.loads(settings.read_text(encoding="utf-8")) assert data["editor.fontSize"] == 14 @@ -119,7 +94,7 @@ def test_setup_preserves_non_utf8_vscode_settings(self, tmp_path, caplog): settings.write_bytes(original) m = IntegrationManifest("copilot", tmp_path) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) assert settings.read_bytes() == original assert "Could not parse" in caplog.text @@ -128,7 +103,7 @@ def test_all_created_files_tracked_in_manifest(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m) + created = copilot.setup(tmp_path, m, parsed_options={"commands": True}) for f in created: rel = f.resolve().relative_to(tmp_path.resolve()).as_posix() assert rel in m.files, f"Created file {rel} not tracked in manifest" @@ -137,7 +112,9 @@ def test_install_uninstall_roundtrip(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - created = copilot.install(tmp_path, m) + created = copilot.install( + tmp_path, m, parsed_options={"commands": True} + ) assert len(created) > 0 m.save() for f in created: @@ -150,7 +127,9 @@ def test_modified_file_survives_uninstall(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - created = copilot.install(tmp_path, m) + created = copilot.install( + tmp_path, m, parsed_options={"commands": True} + ) m.save() modified_file = created[0] modified_file.write_text("user modified this", encoding="utf-8") @@ -162,7 +141,7 @@ def test_directory_structure(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) agents_dir = tmp_path / ".github" / "agents" assert agents_dir.is_dir() agent_files = sorted(agents_dir.glob("speckit.*.agent.md")) @@ -178,7 +157,7 @@ def test_templates_are_processed(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) agents_dir = tmp_path / ".github" / "agents" for agent_file in agents_dir.glob("speckit.*.agent.md"): content = agent_file.read_text(encoding="utf-8") @@ -193,7 +172,7 @@ def test_specify_agent_resolves_active_spec_template(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) specify_file = tmp_path / ".github" / "agents" / "speckit.specify.agent.md" content = specify_file.read_text(encoding="utf-8") @@ -209,7 +188,7 @@ def test_setup_falls_back_to_bundled_command_template_without_preset_override(se copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) specify_file = tmp_path / ".github" / "agents" / "speckit.specify.agent.md" content = specify_file.read_text(encoding="utf-8") @@ -233,7 +212,7 @@ def test_setup_uses_preset_command_override_when_present(self, tmp_path): encoding="utf-8", ) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) specify_file = tmp_path / ".github" / "agents" / "speckit.specify.agent.md" content = specify_file.read_text(encoding="utf-8") @@ -246,14 +225,14 @@ def test_plan_command_has_no_context_placeholder(self, tmp_path): from specify_cli.integrations.copilot import CopilotIntegration copilot = CopilotIntegration() m = IntegrationManifest("copilot", tmp_path) - copilot.setup(tmp_path, m) + copilot.setup(tmp_path, m, parsed_options={"commands": True}) plan_file = tmp_path / ".github" / "agents" / "speckit.plan.agent.md" assert plan_file.exists() content = plan_file.read_text(encoding="utf-8") assert "__CONTEXT_FILE__" not in content def test_complete_file_inventory_sh(self, tmp_path): - """Every file produced by specify init --integration copilot --script sh.""" + """Every file produced by Copilot commands mode with shell scripts.""" from typer.testing import CliRunner from specify_cli import app project = tmp_path / "inventory-sh" @@ -262,7 +241,8 @@ def test_complete_file_inventory_sh(self, tmp_path): try: os.chdir(project) result = CliRunner().invoke(app, [ - "init", "--here", "--integration", "copilot", "--script", "sh", + "init", "--here", "--integration", "copilot", + "--integration-options", "--commands", "--script", "sh", ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -315,7 +295,7 @@ def test_complete_file_inventory_sh(self, tmp_path): ) def test_complete_file_inventory_ps(self, tmp_path): - """Every file produced by specify init --integration copilot --script ps.""" + """Every file produced by Copilot commands mode with PowerShell scripts.""" from typer.testing import CliRunner from specify_cli import app project = tmp_path / "inventory-ps" @@ -324,7 +304,8 @@ def test_complete_file_inventory_ps(self, tmp_path): try: os.chdir(project) result = CliRunner().invoke(app, [ - "init", "--here", "--integration", "copilot", "--script", "ps", + "init", "--here", "--integration", "copilot", + "--integration-options", "--commands", "--script", "ps", ], catch_exceptions=False) finally: os.chdir(old_cwd) @@ -376,54 +357,8 @@ def test_complete_file_inventory_ps(self, tmp_path): f"Extra: {sorted(set(actual) - set(expected))}" ) - def test_default_cli_init_warns_legacy_markdown_is_deprecated(self, tmp_path): - """Default Copilot init should warn users about the future skills default.""" - from typer.testing import CliRunner - from specify_cli import app - project = tmp_path / "default-warning" - project.mkdir() - old_cwd = os.getcwd() - try: - os.chdir(project) - with pytest.warns( - UserWarning, - match="Copilot legacy markdown mode is deprecated", - ): - result = CliRunner().invoke(app, [ - "init", "--here", "--integration", "copilot", "--script", "sh", - ], catch_exceptions=False) - finally: - os.chdir(old_cwd) - - assert result.exit_code == 0, result.output - - def test_skills_cli_init_does_not_warn_about_legacy_markdown(self, tmp_path): - """Explicit Copilot skills mode should not warn about the legacy default.""" - from typer.testing import CliRunner - from specify_cli import app - project = tmp_path / "skills-no-warning" - project.mkdir() - old_cwd = os.getcwd() - try: - os.chdir(project) - with warnings.catch_warnings(record=True) as caught: - warnings.simplefilter("always") - result = CliRunner().invoke(app, [ - "init", "--here", "--integration", "copilot", - "--integration-options", "--skills", "--script", "sh", - ], catch_exceptions=False) - finally: - os.chdir(old_cwd) - - assert result.exit_code == 0, result.output - assert not any( - "Copilot legacy markdown mode is deprecated" in str(item.message) - for item in caught - ) - - class TestCopilotSkillsMode: - """Tests for Copilot integration in --skills mode.""" + """Tests for Copilot's default skills mode.""" _SKILL_COMMANDS = [ "analyze", "clarify", "constitution", "converge", "implement", @@ -436,7 +371,7 @@ def _make_copilot(self): def _setup_skills(self, copilot, tmp_path): m = IntegrationManifest("copilot", tmp_path) - created = copilot.setup(tmp_path, m, parsed_options={"skills": True}) + created = copilot.setup(tmp_path, m) return created, m # -- Options ---------------------------------------------------------- @@ -449,6 +384,137 @@ def test_options_include_skills_flag(self): assert skills_opts[0].is_flag is True assert skills_opts[0].default is False + def test_options_include_commands_flag(self): + copilot = get_integration("copilot") + commands_opts = [o for o in copilot.options() if o.name == "--commands"] + assert len(commands_opts) == 1 + assert commands_opts[0].is_flag is True + assert commands_opts[0].default is False + + def test_default_is_skills_mode(self): + copilot = self._make_copilot() + assert copilot.is_skills_mode() is True + assert copilot.is_skills_mode({}) is True + + def test_commands_flag_disables_skills_mode(self): + copilot = self._make_copilot() + assert copilot.is_skills_mode({"commands": True}) is False + + def test_existing_commands_layout_is_preserved(self, tmp_path): + copilot = self._make_copilot() + agents_dir = tmp_path / ".github" / "agents" + agents_dir.mkdir(parents=True) + (agents_dir / "speckit.plan.agent.md").write_text( + "# plan\n", encoding="utf-8" + ) + assert copilot.is_skills_mode(project_root=tmp_path) is False + + def test_setup_preserves_existing_commands_without_stored_options( + self, tmp_path + ): + copilot = self._make_copilot() + agents_dir = tmp_path / ".github" / "agents" + agents_dir.mkdir(parents=True) + (agents_dir / "speckit.plan.agent.md").write_text( + "# old plan\n", encoding="utf-8" + ) + manifest = IntegrationManifest("copilot", tmp_path) + + created = copilot.setup(tmp_path, manifest) + + assert any(path.name.endswith(".agent.md") for path in created) + assert not (tmp_path / ".github" / "skills").exists() + assert copilot._skills_mode is False + + def test_existing_skills_layout_stays_in_skills_mode(self, tmp_path): + copilot = self._make_copilot() + (tmp_path / ".github" / "skills" / "speckit-plan").mkdir(parents=True) + assert copilot.is_skills_mode(project_root=tmp_path) is True + + def test_commands_manifest_wins_over_untracked_skill(self, tmp_path): + copilot = self._make_copilot() + manifest = IntegrationManifest("copilot", tmp_path) + copilot.setup( + tmp_path, manifest, parsed_options={"commands": True} + ) + manifest.save() + stale_skill = ( + tmp_path + / ".github" + / "skills" + / "speckit-plan" + / "SKILL.md" + ) + stale_skill.parent.mkdir(parents=True) + stale_skill.write_text("# user-authored skill\n", encoding="utf-8") + + assert copilot.is_skills_mode(project_root=tmp_path) is False + + def test_skills_manifest_wins_over_untracked_command(self, tmp_path): + copilot = self._make_copilot() + manifest = IntegrationManifest("copilot", tmp_path) + copilot.setup(tmp_path, manifest) + manifest.save() + stale_agent = ( + tmp_path + / ".github" + / "agents" + / "speckit.plan.agent.md" + ) + stale_agent.parent.mkdir(parents=True) + stale_agent.write_text("# stale command\n", encoding="utf-8") + + assert copilot.is_skills_mode(project_root=tmp_path) is True + + def test_explicit_skills_forces_migration_from_commands(self, tmp_path): + copilot = self._make_copilot() + agents_dir = tmp_path / ".github" / "agents" + agents_dir.mkdir(parents=True) + (agents_dir / "speckit.plan.agent.md").write_text( + "# plan\n", encoding="utf-8" + ) + assert ( + copilot.is_skills_mode({"skills": True}, project_root=tmp_path) + is True + ) + + def test_skills_and_commands_flags_are_mutually_exclusive(self): + copilot = self._make_copilot() + with pytest.raises(typer.Exit): + copilot.is_skills_mode({"skills": True, "commands": True}) + + def test_cli_rejects_skills_and_commands_together(self, tmp_path): + from typer.testing import CliRunner + + from specify_cli import app + + project = tmp_path / "conflicting-modes" + project.mkdir() + old_cwd = os.getcwd() + try: + os.chdir(project) + result = CliRunner().invoke( + app, + [ + "init", + "--here", + "--integration", + "copilot", + "--integration-options", + "--skills --commands", + "--script", + "sh", + ], + catch_exceptions=False, + ) + finally: + os.chdir(old_cwd) + + assert result.exit_code == 1 + assert "--skills and --commands are mutually exclusive" in result.output + assert not (project / ".github" / "skills").exists() + assert not (project / ".github" / "agents").exists() + # -- Skills directory structure --------------------------------------- def test_skills_creates_skill_files(self, tmp_path): @@ -623,16 +689,16 @@ def test_skills_command_refs_use_hyphen(self, tmp_path): def test_skills_mode_invoke_separator(self): """Copilot effective_invoke_separator should reflect skills mode.""" copilot = self._make_copilot() - assert copilot.effective_invoke_separator() == "." + assert copilot.effective_invoke_separator() == "-" assert copilot.effective_invoke_separator({"skills": True}) == "-" - assert copilot.effective_invoke_separator({"skills": False}) == "." + assert copilot.effective_invoke_separator({"commands": True}) == "." def test_invoke_separator_for_mode_tracks_persisted_state(self): """Regression (review #3415): registration paths (preset/extension command refs) must resolve the separator from the persisted ai_skills state. A Copilot skills project renders ``/speckit-`` (hyphen), - matching ``build_command_invocation``; the default markdown layout - renders ``/speckit.`` (dot). + matching ``build_command_invocation``; commands mode renders + ``/speckit.`` (dot). """ copilot = self._make_copilot() assert copilot.invoke_separator_for_mode(True) == "-" @@ -673,7 +739,7 @@ def test_all_files_tracked_in_manifest(self, tmp_path): def test_install_uninstall_roundtrip(self, tmp_path): copilot = self._make_copilot() m = IntegrationManifest("copilot", tmp_path) - created = copilot.install(tmp_path, m, parsed_options={"skills": True}) + created = copilot.install(tmp_path, m) assert len(created) > 0 m.save() for f in created: @@ -685,7 +751,7 @@ def test_install_uninstall_roundtrip(self, tmp_path): def test_modified_file_survives_uninstall(self, tmp_path): copilot = self._make_copilot() m = IntegrationManifest("copilot", tmp_path) - created = copilot.install(tmp_path, m, parsed_options={"skills": True}) + created = copilot.install(tmp_path, m) m.save() modified_file = created[0] modified_file.write_text("user modified this", encoding="utf-8") @@ -710,6 +776,12 @@ def test_build_command_invocation_skills_extension_command(self): def test_build_command_invocation_default_mode(self): copilot = self._make_copilot() + assert copilot.build_command_invocation("plan", "my args") == "/speckit-plan my args" + assert copilot.build_command_invocation("plan") == "/speckit-plan" + + def test_build_command_invocation_commands_mode(self): + copilot = self._make_copilot() + copilot._skills_mode = False assert copilot.build_command_invocation("plan", "my args") == "my args" assert copilot.build_command_invocation("plan") == "" @@ -725,8 +797,8 @@ def test_skills_setup_does_not_write_context_section(self, tmp_path): # -- CLI integration test --------------------------------------------- - def test_init_with_integration_options_skills(self, tmp_path): - """specify init --integration copilot --integration-options='--skills' scaffolds skills.""" + def test_init_defaults_to_skills(self, tmp_path): + """specify init --integration copilot scaffolds skills by default.""" from typer.testing import CliRunner from specify_cli import app project = tmp_path / "copilot-skills" @@ -736,7 +808,6 @@ def test_init_with_integration_options_skills(self, tmp_path): os.chdir(project) result = CliRunner().invoke(app, [ "init", "--here", "--integration", "copilot", - "--integration-options", "--skills", "--script", "sh", ], catch_exceptions=False) finally: @@ -752,7 +823,7 @@ def test_init_with_integration_options_skills(self, tmp_path): assert not (project / ".vscode" / "settings.json").exists() def test_complete_file_inventory_skills_sh(self, tmp_path): - """Every file produced by specify init --integration copilot --integration-options='--skills' --script sh.""" + """Every file produced by default Copilot init with shell scripts.""" from typer.testing import CliRunner from specify_cli import app project = tmp_path / "inventory-skills-sh" @@ -762,7 +833,6 @@ def test_complete_file_inventory_skills_sh(self, tmp_path): os.chdir(project) result = CliRunner().invoke(app, [ "init", "--here", "--integration", "copilot", - "--integration-options", "--skills", "--script", "sh", ], catch_exceptions=False) finally: @@ -802,36 +872,46 @@ def test_complete_file_inventory_skills_sh(self, tmp_path): # -- Singleton leak: _skills_mode must reset -------------------------- - def test_skills_mode_resets_on_default_setup(self, tmp_path): - """setup() with skills=True then without must reset _skills_mode.""" + def test_skills_mode_resets_between_layouts(self, tmp_path): + """setup() must reset the singleton mode for each selected layout.""" copilot = self._make_copilot() - # First call: skills mode + # First call: default skills mode (tmp_path / "proj1").mkdir() m1 = IntegrationManifest("copilot", tmp_path / "proj1") - copilot.setup(tmp_path / "proj1", m1, parsed_options={"skills": True}) + copilot.setup(tmp_path / "proj1", m1) assert copilot._skills_mode is True - # Second call: default mode (no skills option) + # Second call: explicit commands mode (tmp_path / "proj2").mkdir() m2 = IntegrationManifest("copilot", tmp_path / "proj2") - copilot.setup(tmp_path / "proj2", m2) + copilot.setup( + tmp_path / "proj2", m2, parsed_options={"commands": True} + ) assert copilot._skills_mode is False - - # build_command_invocation must use default (dotted) mode assert copilot.build_command_invocation("plan", "args") == "args" - # -- Auto-detection must ignore unrelated .github/skills/ ------------- + # Third call: a fresh default project must switch back to skills. + (tmp_path / "proj3").mkdir() + m3 = IntegrationManifest("copilot", tmp_path / "proj3") + copilot.setup(tmp_path / "proj3", m3) + assert copilot._skills_mode is True + assert copilot.build_command_invocation("plan") == "/speckit-plan" + + # -- Auto-detection must preserve managed commands -------------------- - def test_dispatch_ignores_unrelated_skills_directory(self, tmp_path): - """dispatch_command() must not treat unrelated .github/skills/ as skills mode.""" + def test_dispatch_preserves_commands_with_unrelated_skills(self, tmp_path): + """Unrelated skills must not migrate a managed commands layout.""" copilot = self._make_copilot() - # Create a .github/skills/ with non-speckit content (e.g. GitHub Skills training) + agents_dir = tmp_path / ".github" / "agents" + agents_dir.mkdir(parents=True) + (agents_dir / "speckit.plan.agent.md").write_text( + "# plan\n", encoding="utf-8" + ) unrelated = tmp_path / ".github" / "skills" / "introduction-to-github" unrelated.mkdir(parents=True) (unrelated / "README.md").write_text("# GitHub Skills training\n") - # Should NOT detect skills mode — cli_args should contain --agent import unittest.mock as mock with mock.patch("subprocess.run") as mock_run: mock_run.return_value = mock.Mock(returncode=0, stdout="", stderr="") @@ -868,7 +948,7 @@ def test_dispatch_detects_speckit_skills_layout(self, tmp_path): # -- Next-steps display for Copilot skills mode ----------------------- def test_init_skills_next_steps_show_skill_syntax(self, tmp_path): - """specify init --integration copilot --integration-options='--skills' shows /speckit-plan not /speckit.plan.""" + """Default Copilot init shows /speckit-plan, not /speckit.plan.""" from typer.testing import CliRunner from specify_cli import app project = tmp_path / "copilot-nextsteps" @@ -878,7 +958,6 @@ def test_init_skills_next_steps_show_skill_syntax(self, tmp_path): os.chdir(project) result = CliRunner().invoke(app, [ "init", "--here", "--integration", "copilot", - "--integration-options", "--skills", ], catch_exceptions=False) finally: os.chdir(old_cwd) diff --git a/tests/integrations/test_integration_state.py b/tests/integrations/test_integration_state.py index fc12d436a4..ebedc1056c 100644 --- a/tests/integrations/test_integration_state.py +++ b/tests/integrations/test_integration_state.py @@ -89,10 +89,10 @@ def test_write_integration_json_strips_integration_key(tmp_path): def test_with_integration_setting_recomputes_separator_from_retained_options(): """Updating only script_type must not drop an options-dependent separator. - Copilot resolves the command-ref separator to '-' when '--skills' options - are stored and '.' otherwise. A second call that changes only script_type + Copilot resolves the command-ref separator to '.' when '--commands' is + stored and '-' by default. A second call that changes only script_type (parsed_options=None, raw_options=None) retains the stored parsed_options, - so invoke_separator must stay '-', not be recomputed from the None argument. + so invoke_separator must stay '.', not be recomputed from the None argument. """ from specify_cli.integrations import get_integration from specify_cli.integration_runtime import with_integration_setting @@ -100,15 +100,15 @@ def test_with_integration_setting_recomputes_separator_from_retained_options(): copilot = get_integration("copilot") settings = with_integration_setting( - {}, "copilot", copilot, parsed_options={"skills": True} + {}, "copilot", copilot, parsed_options={"commands": True} ) - assert settings["copilot"]["invoke_separator"] == "-" + assert settings["copilot"]["invoke_separator"] == "." settings2 = with_integration_setting( {"integration_settings": settings}, "copilot", copilot, script_type="ps" ) # parsed_options are retained (only script_type changed) ... - assert settings2["copilot"]["parsed_options"] == {"skills": True} + assert settings2["copilot"]["parsed_options"] == {"commands": True} assert settings2["copilot"]["script"] == "ps" # ... so the separator must reflect them, not the (None) argument. - assert settings2["copilot"]["invoke_separator"] == "-" + assert settings2["copilot"]["invoke_separator"] == "." diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 32753d1cda..994fecb148 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -2224,15 +2224,99 @@ def test_switch_between_integrations(self, tmp_path): # Old claude files removed assert not (project / ".claude" / "skills" / "speckit-plan" / "SKILL.md").exists() - # New copilot files created - assert (project / ".github" / "agents" / "speckit.plan.agent.md").exists() - assert "/speckit.specify" in shared_script.read_text(encoding="utf-8") - assert "/speckit-specify" not in shared_script.read_text(encoding="utf-8") + # New default Copilot skills created + assert ( + project / ".github" / "skills" / "speckit-plan" / "SKILL.md" + ).exists() + assert "/speckit-specify" in shared_script.read_text(encoding="utf-8") + assert "/speckit.specify" not in shared_script.read_text(encoding="utf-8") # integration.json updated data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) assert data["integration"] == "copilot" + def test_switch_rejects_conflicting_copilot_modes_before_uninstall( + self, tmp_path + ): + project = _init_project(tmp_path, "claude") + claude_skill = ( + project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + ) + before_state = json.loads( + (project / ".specify" / "integration.json").read_text( + encoding="utf-8" + ) + ) + + result = _run_in_project( + project, + [ + "integration", + "switch", + "copilot", + "--integration-options", + "--skills --commands", + "--script", + "sh", + ], + ) + + assert result.exit_code == 1 + assert "--skills and --commands are mutually exclusive" in result.output + assert claude_skill.exists() + assert not (project / ".github" / "skills").exists() + assert not (project / ".github" / "agents").exists() + after_state = json.loads( + (project / ".specify" / "integration.json").read_text( + encoding="utf-8" + ) + ) + assert after_state == before_state + + def test_switch_preserves_target_options_with_fallback_integration( + self, tmp_path + ): + project = _init_project(tmp_path, "claude") + install = _run_in_project( + project, + [ + "integration", + "install", + "opencode", + "--script", + "sh", + "--force", + ], + ) + assert install.exit_code == 0, install.output + + result = _run_in_project( + project, + [ + "integration", + "switch", + "copilot", + "--integration-options", + "--commands", + "--script", + "sh", + ], + ) + + assert result.exit_code == 0, result.output + assert ( + project / ".github" / "agents" / "speckit.plan.agent.md" + ).exists() + assert not (project / ".github" / "skills").exists() + state = json.loads( + (project / ".specify" / "integration.json").read_text( + encoding="utf-8" + ) + ) + assert state["integration_settings"]["copilot"]["parsed_options"] == { + "commands": True + } + def test_switch_migrates_extension_commands(self, tmp_path): """Switching should migrate extension commands to the new agent directory.""" project = _init_project(tmp_path, "kimi") @@ -2492,6 +2576,7 @@ def test_switch_refreshes_managed_shared_script_refs(self, tmp_path): os.chdir(project) result = runner.invoke(app, [ "integration", "switch", "copilot", + "--integration-options", "--commands", "--script", "sh", ], catch_exceptions=False) finally: @@ -2530,6 +2615,7 @@ def test_switch_refreshes_stale_managed_shared_infra(self, tmp_path): os.chdir(project) result = runner.invoke(app, [ "integration", "switch", "copilot", + "--integration-options", "--commands", "--script", "sh", ], catch_exceptions=False) finally: @@ -2558,6 +2644,7 @@ def test_switch_preserves_user_customized_shared_infra(self, tmp_path): os.chdir(project) result = runner.invoke(app, [ "integration", "switch", "copilot", + "--integration-options", "--commands", "--script", "sh", ], catch_exceptions=False) finally: @@ -2582,6 +2669,7 @@ def test_switch_refresh_shared_infra_overwrites_customizations(self, tmp_path): os.chdir(project) result = runner.invoke(app, [ "integration", "switch", "copilot", + "--integration-options", "--commands", "--script", "sh", "--refresh-shared-infra", ], catch_exceptions=False) @@ -2894,7 +2982,9 @@ def fail_refresh(*args, **kwargs): assert manifest_path.read_text(encoding="utf-8") == before_manifest def test_upgrade_default_refreshes_shared_script_refs_for_option_separator_change(self, tmp_path): - project = _init_project(tmp_path, "copilot") + project = _init_project( + tmp_path, "copilot", integration_options="--commands" + ) template = project / ".specify" / "templates" / "plan-template.md" managed_script = project / ".specify" / "scripts" / "bash" / "check-prerequisites.sh" customized_script = project / ".specify" / "scripts" / "bash" / "setup-tasks.sh" @@ -2916,6 +3006,46 @@ def test_upgrade_default_refreshes_shared_script_refs_for_option_separator_chang assert "/speckit.specify" not in managed_content assert customized_script.read_text(encoding="utf-8") == customized_before + def test_upgrade_preserves_historical_copilot_commands_without_options( + self, tmp_path + ): + """A command manifest restores missing files instead of migrating.""" + project = _init_project( + tmp_path, "copilot", integration_options="--commands" + ) + state_path = project / ".specify" / "integration.json" + state = json.loads(state_path.read_text(encoding="utf-8")) + copilot_settings = state["integration_settings"]["copilot"] + copilot_settings.pop("raw_options", None) + copilot_settings.pop("parsed_options", None) + state_path.write_text(json.dumps(state), encoding="utf-8") + + for path in (project / ".github" / "agents").glob( + "speckit.*.agent.md" + ): + path.unlink() + for path in (project / ".github" / "prompts").glob( + "speckit.*.prompt.md" + ): + path.unlink() + + result = _run_in_project( + project, + ["integration", "upgrade", "copilot", "--script", "sh", "--force"], + ) + + assert result.exit_code == 0, result.output + assert ( + project / ".github" / "agents" / "speckit.plan.agent.md" + ).exists() + assert not (project / ".github" / "skills").exists() + init_options = json.loads( + (project / ".specify" / "init-options.json").read_text( + encoding="utf-8" + ) + ) + assert init_options.get("ai_skills") is not True + def test_upgrade_non_default_keeps_default_template_invocations(self, tmp_path): project = _init_project(tmp_path, "gemini") template = project / ".specify" / "templates" / "plan-template.md" @@ -3721,7 +3851,9 @@ def test_upgrade_preserves_existing_vscode_settings(self, tmp_path): tracking it, so without ``stale_cleanup_exclusions()`` the Phase 2 stale cleanup would delete it (destroying the user's settings). """ - project = _init_project(tmp_path, "copilot") + project = _init_project( + tmp_path, "copilot", integration_options="--commands" + ) settings = project / ".vscode" / "settings.json" assert settings.is_file(), "init should create .vscode/settings.json" before = json.loads(settings.read_text(encoding="utf-8")) diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index d2941f5dc3..6eec5e7b47 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -2043,7 +2043,7 @@ def test_rescaffold_toggle_skills_to_command_removes_stale_extension_skill_file( assert skill_file.exists(), "sanity: skills mode should write SKILL.md" # Toggle ai_skills off for the same active agent (copilot) and - # rescaffold, mirroring `integration upgrade copilot` (no --skills). + # rescaffold, mirroring `integration upgrade copilot --commands`. _create_init_options(project_dir, ai="copilot", ai_skills=False) manager.register_enabled_extensions_for_agent("copilot") @@ -2117,7 +2117,7 @@ def test_toggle_to_command_preserves_tracking_for_mirror_in_other_agent_dir( ) # Toggle copilot to command mode (mirroring `integration upgrade - # copilot` with no --skills) — copilot's mirror is now stale. + # copilot --commands`) — copilot's mirror is now stale. _create_init_options(project_dir, ai="copilot", ai_skills=False) manager.register_enabled_extensions_for_agent("copilot")