From 6bb1846fdf3c09410c75d46959d1985352f29b35 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Thu, 30 Apr 2026 08:26:57 +0200 Subject: [PATCH 1/5] feat(extensions): neutral behavior vocabulary, invocability fix, and extension path rewriting --- .gitignore | 6 + .../RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md | 223 +++++ extensions/RFC-EXTENSION-SYSTEM.md | 13 + src/specify_cli/__init__.py | 31 +- src/specify_cli/agents.py | 257 +++++- src/specify_cli/behavior.py | 197 ++++ src/specify_cli/extensions.py | 73 +- src/specify_cli/integrations/base.py | 40 + .../integrations/claude/__init__.py | 4 +- templates/commands/analyze.md | 2 + templates/commands/checklist.md | 2 + templates/commands/clarify.md | 2 + templates/commands/constitution.md | 2 + templates/commands/implement.md | 2 + templates/commands/plan.md | 2 + templates/commands/specify.md | 2 + templates/commands/tasks.md | 2 + templates/commands/taskstoissues.md | 2 + tests/integrations/test_integration_claude.py | 1 + tests/test_agent_deployment.py | 587 ++++++++++++ tests/test_behavior_translator.py | 215 +++++ tests/test_extension_skills.py | 236 +++++ tests/test_extensions.py | 283 ++++++ .../test_integration_extension_skill_paths.py | 214 +++++ tests/test_pr2103_reviews.py | 873 ++++++++++++++++++ 25 files changed, 3235 insertions(+), 36 deletions(-) create mode 100644 extensions/RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md create mode 100644 src/specify_cli/behavior.py create mode 100644 tests/test_agent_deployment.py create mode 100644 tests/test_behavior_translator.py create mode 100644 tests/test_integration_extension_skill_paths.py create mode 100644 tests/test_pr2103_reviews.py diff --git a/.gitignore b/.gitignore index 1688c8299e..c2896705c0 100644 --- a/.gitignore +++ b/.gitignore @@ -33,6 +33,10 @@ env/ *.swo .DS_Store *.tmp +.venv312 +.specify +.gitignore +.claude # Project specific *.log @@ -45,6 +49,8 @@ env/ *.zip sdd-*/ docs/dev +specs + # Extension system .specify/extensions/.cache/ diff --git a/extensions/RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md b/extensions/RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md new file mode 100644 index 0000000000..446f6226d1 --- /dev/null +++ b/extensions/RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md @@ -0,0 +1,223 @@ +# Extension Behavior & Deployment — RFC Addendum + +## Overview + +Extension commands can declare two new frontmatter sections: + +1. **`behavior:`** — agent-neutral intent vocabulary +2. **`agents:`** — per-agent escape hatch for fields with no neutral equivalent + +Deployment target is fully derived from `behavior.execution` — no separate manifest field is needed. + +--- + +## `behavior:` Vocabulary + +```yaml +behavior: + execution: command | isolated | agent + capability: fast | balanced | strong + effort: low | medium | high | max + tools: none | read-only | write | full | + invocation: explicit | automatic + visibility: user | model | both + color: red | blue | green | yellow | purple | orange | pink | cyan +``` + +### Per-agent translation + +| behavior field | value | Claude | Copilot | Codex | Others | +|---|---|---|---|---|---| +| `execution` | `isolated` | `context: fork` | — | — | — | +| `execution` | `agent` | routing only (see Deployment section) | — | — | — | +| `capability` | `fast` | `model: claude-haiku-4-5-20251001` | `model: Claude Haiku 4.5` | — | — | +| `capability` | `balanced` | `model: claude-sonnet-4-6` | `model: Claude Sonnet 4.5` | — | — | +| `capability` | `strong` | `model: claude-opus-4-6` | `model: Claude Opus 4.5` | — | — | +| `effort` | any | `effort: {value}` | — | `effort: {value}` | — | +| `tools` | `read-only` | `allowed-tools: Read Grep Glob` | `tools: [read_file, list_directory, search_files]` | — | — | +| `tools` | `write` | `allowed-tools: Read Write Edit Grep Glob` | `tools: ["*"]` | — | — | +| `tools` | `none` | `allowed-tools: ""` | `tools: []` | — | — | +| `tools` | `full` | — (no restriction, all tools available) | `tools: ["*"]` | — | — | +| `tools` | `` | `allowed-tools: ` (literal passthrough) | — | — | — | +| `tools` | `` | `allowed-tools: ` | — | — | — | +| `invocation` | `explicit` | `disable-model-invocation: true` | `disable-model-invocation: true` | — | — | +| `invocation` | `automatic` | `disable-model-invocation: false` | `disable-model-invocation: false` | — | — | +| `visibility` | `user` | `user-invocable: true` | `user-invocable: true` | — | — | +| `visibility` | `model` | `user-invocable: false` | `user-invocable: false` | — | — | +| `visibility` | `both` | — | — | — | — | +| `color` | any valid value | `color: {value}` | — | — | — | + +Cells marked `—` mean "no concept, field omitted silently." + +> **Note:** For Claude agent definitions (`execution: agent`), the `allowed-tools` key is automatically remapped to `tools` by spec-kit during deployment. The table above shows the `allowed-tools` form used in skill files (SKILL.md); the agent definition example below shows the resulting `tools` key after remapping. + +### `tools` presets and custom values (Claude) + +The `tools` field accepts four named presets or a custom value: + +| value | `allowed-tools` written | use case | +|---|---|---| +| `none` | `""` (empty — no tools) | pure reasoning, no file access | +| `read-only` | `Read Grep Glob` | read/search, no writes | +| `write` | `Read Write Edit Grep Glob` | file reads + writes, no shell | +| `full` | _(key omitted)_ | all tools including Bash | + +For anything outside these presets, pass a **custom string** or **YAML list** — it is written verbatim as `allowed-tools`: + +```yaml +# Custom string (space-separated) +behavior: + tools: "Read Write Bash" + +# YAML list (joined with spaces) +behavior: + tools: + - Read + - Write + - Bash +``` + +> Custom values bypass preset lookup entirely and are not validated. Use named presets whenever possible. + +### `color` (Claude Code only) + +Controls the UI color of the agent entry in the Claude Code task list and transcript. Accepted values: `red`, `blue`, `green`, `yellow`, `purple`, `orange`, `pink`, `cyan`. The value is passed through verbatim to the agent definition frontmatter — no translation occurs. Other agents ignore this field. + +--- + +## `agents:` Escape Hatch + +For fields with no neutral equivalent, declare them per-agent: + +```yaml +agents: + claude: + paths: "src/**" + argument-hint: "Path to the codebase" + copilot: + someCustomKey: someValue +``` + +Agent-specific overrides win over `behavior:` translations. + +--- + +## Deployment Routing from `behavior.execution` + +Deployment target is fully derived from `behavior.execution` in the command file — no separate manifest field needed. + +| `behavior.execution` | Claude | Copilot | Codex | Others | +|---|---|---|---|---| +| `command` (default) | `.claude/skills/{name}/SKILL.md` | `.github/agents/{name}.agent.md` | `.agents/skills/{name}/SKILL.md` | per-agent format | +| `isolated` | `.claude/skills/{name}/SKILL.md` + `context: fork` | `.github/agents/{name}.agent.md` + `mode: agent` | per-agent format | per-agent format | +| `agent` | `.claude/agents/{name}.md` | `.github/agents/{name}.agent.md` + `mode: agent` + `tools:` | not supported | not supported | + +### Agent definition format (Claude, `execution: agent`) + +Spec-kit writes a Claude agent definition file at `.claude/agents/{name}.md`. +The body becomes the **system prompt**. Frontmatter is minimal — no +`user-invocable`, `disable-model-invocation`, `context`, or `metadata` keys. + +```markdown +--- +name: speckit-revenge-analyzer +description: Codebase analyzer subagent +model: claude-opus-4-6 +tools: Read Grep Glob +--- +You are a codebase analysis specialist... +``` + +### Deferred: `execution: isolated` as agent definition + +It is theoretically possible to want a command that runs in an isolated +context (`context: fork`) AND is deployed as a named agent definition +(`.claude/agents/`). These two concerns are orthogonal — isolation is a +runtime concern, agent definition is a deployment concern. + +This combination is **not supported** in this implementation. `execution: +isolated` always deploys as a skill file. Decoupling runtime context from +deployment target is deferred until a concrete use case requires it. + +--- + +## Full Example: Orchestrator + Reusable Subagent + +**`extension.yml`** (no manifest `type` field — deployment derived from command frontmatter): +```yaml +provides: + commands: + - name: speckit.revenge.extract + file: commands/extract.md + + - name: speckit.revenge.analyzer + file: commands/analyzer.md +``` + +**`commands/extract.md`** (orchestrator skill — no `execution:` → deploys to skills): +```markdown +--- +description: Run the extraction pipeline +behavior: + invocation: automatic +agents: + claude: + argument-hint: "Path to codebase (optional)" +--- +Orchestrate extraction for $ARGUMENTS... +``` + +**`commands/analyzer.md`** (reusable subagent — `execution: agent` → deploys to `.claude/agents/`): +```markdown +--- +description: Analyze codebase structure and extract domain information +behavior: + execution: agent + capability: strong + tools: read-only + color: green +agents: + claude: + paths: "src/**" +--- +You are a codebase analysis specialist. +Analyze $ARGUMENTS and return structured domain findings. +``` + +The deployed `.claude/agents/speckit-revenge-analyzer.md` will contain: + +```markdown +--- +name: speckit-revenge-analyzer +description: Analyze codebase structure and extract domain information +model: claude-opus-4-6 +tools: Read Grep Glob +color: green +--- +You are a codebase analysis specialist. +... +``` + +### `tools: write` example + +Use `write` when an agent needs to create or modify files but does not need shell access (Bash): + +```yaml +behavior: + execution: agent + capability: strong + tools: write # Read Write Edit Grep Glob — no Bash + color: yellow +``` + +### `tools: full` example + +Use `full` when an agent needs unrestricted access including Bash (running tests, git commands, CLI tools): + +```yaml +behavior: + execution: agent + capability: strong + tools: full # all tools; no allowed-tools key injected + color: red +``` diff --git a/extensions/RFC-EXTENSION-SYSTEM.md b/extensions/RFC-EXTENSION-SYSTEM.md index dd4c97e8a2..6c4b87cc30 100644 --- a/extensions/RFC-EXTENSION-SYSTEM.md +++ b/extensions/RFC-EXTENSION-SYSTEM.md @@ -27,6 +27,7 @@ 16. [Resolved Questions](#resolved-questions) 17. [Open Questions (Remaining)](#open-questions-remaining) 18. [Appendices](#appendices) +19. [RFC Addenda](#rfc-addenda) --- @@ -597,6 +598,8 @@ def convert_to_claude( dest.write_text(render_frontmatter(frontmatter) + "\n" + body) ``` +> **See also:** [RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md](RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md) — addendum covering agent-neutral `behavior:` vocabulary, per-agent translation, `agents:` escape hatch, and deployment routing (`behavior.execution: agent` → `.claude/agents/`). + --- ## Configuration Management @@ -1960,3 +1963,13 @@ This RFC proposes a comprehensive extension system for Spec Kit that: 3. Should we support extension dependencies (extension A requires extension B)? 4. How should we handle extension deprecation/removal from catalog? 5. What level of sandboxing/permissions do we need in v1.0? + +--- + +## RFC Addenda + +Addenda extend this RFC with post-initial-implementation decisions. They are authoritative and supersede any conflicting content in the main RFC. + +| Addendum | Topic | Status | +|---|---|---| +| [RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md](RFC-EXTENSION-BEHAVIOR-DEPLOYMENT.md) | Agent-neutral `behavior:` vocabulary, deployment routing from `behavior.execution`, `agents:` escape hatch | Implemented (2026-04-08) | diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index f5e117beef..dae3f76e6d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -4568,12 +4568,13 @@ def extension_update( shutil.rmtree(backup_ext_dir) shutil.copytree(extension_dir, backup_ext_dir) - # Backup config files separately so they can be restored - # after a successful install (install_from_directory clears dest dir). - config_files = list(extension_dir.glob("*-config.yml")) + list( - extension_dir.glob("*-config.local.yml") - ) - for cfg_file in config_files: + # Backup all top-level .yml files (except extension.yml) so they + # can be restored after install. Covers user configs (*-config.yml, + # *-config.local.yml) and any runtime-generated state files that + # extensions write to their own directory. + for cfg_file in extension_dir.glob("*.yml"): + if cfg_file.name == "extension.yml": + continue backup_config_dir.mkdir(parents=True, exist_ok=True) shutil.copy2(cfg_file, backup_config_dir / cfg_file.name) @@ -4652,12 +4653,24 @@ def extension_update( # 8. Install new version _ = manager.install_from_zip(zip_path, speckit_version) - # Restore user config files from backup after successful install. + # Restore backed-up files after successful install. + # User config files (*-config.yml, *-config.local.yml) always + # overwrite the new extension defaults — user settings win. + # All other .yml files (runtime-generated state) are restored + # only when the new extension version didn't ship that file, + # so updated source files are never rolled back. new_extension_dir = manager.extensions_dir / extension_id if backup_config_dir.exists() and new_extension_dir.exists(): for cfg_file in backup_config_dir.iterdir(): - if cfg_file.is_file(): - shutil.copy2(cfg_file, new_extension_dir / cfg_file.name) + if not cfg_file.is_file(): + continue + dest = new_extension_dir / cfg_file.name + is_user_config = ( + cfg_file.name.endswith("-config.yml") + or cfg_file.name.endswith("-config.local.yml") + ) + if is_user_config or not dest.exists(): + shutil.copy2(cfg_file, dest) # 9. Restore metadata from backup (installed_at, enabled state) if backup_registry_entry and isinstance(backup_registry_entry, dict): diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 726b0fd2a6..55504b2ee5 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -15,6 +15,31 @@ from copy import deepcopy import yaml +from specify_cli.behavior import translate_behavior, strip_behavior_keys, get_deployment_type, get_copilot_tools + + +# Agent-specific frontmatter keys that extension/preset authors may declare in +# source command frontmatter and have passed through verbatim to the generated +# skill file. Keys not in this set are ignored during skill rendering. +_SKILL_PASSTHROUGH_KEYS: dict[str, frozenset[str]] = { + "claude": frozenset({ + "context", # fork execution model + "agent", # subagent type when context: fork + "model", # model override + "effort", # effort level + "allowed-tools", # tool restriction list + "color", # UI color in Claude Code task list + "paths", # path-based activation glob + "argument-hint", # UI hint in slash-command menu + "disable-model-invocation", # override default True + "user-invocable", # override default True + }), + "codex": frozenset({ + "model", + "effort", + }), +} + def _build_agent_configs() -> dict[str, Any]: """Derive CommandRegistrar.AGENT_CONFIGS from INTEGRATION_REGISTRY.""" @@ -155,6 +180,54 @@ def rewrite_project_relative_paths(text: str) -> str: ".specify.specify/", ".specify/" ) + @staticmethod + def rewrite_extension_paths(text: str, extension_id: str, extension_dir: Path) -> str: + """Rewrite extension-relative paths to their installed project locations. + + Extension command bodies reference files using paths relative to the + extension root (e.g. ``agents/control/commander.md``). After install, + those files live at ``.specify/extensions//...``. This method + rewrites such references so that AI agents can locate them after install. + + Only directories that actually exist inside *extension_dir* are rewritten, + keeping the behaviour conservative and avoiding false positives on prose. + + Args: + text: Body text of the command file. + extension_id: The extension identifier (e.g. ``"echelon"``). + extension_dir: Path to the installed extension directory. + + Returns: + Body text with extension-relative paths expanded. + """ + if not isinstance(text, str) or not text: + return text + + _SKIP = {"commands", ".git", "specs"} + try: + subdirs = [ + d.name + for d in extension_dir.iterdir() + if d.is_dir() and d.name not in _SKIP and not d.name.startswith(".") + ] + except OSError: + return text + + base_prefix = f".specify/extensions/{extension_id}/" + + # Replace $EXTENSION_PATH shell variable with the actual installed path. + text = text.replace("$EXTENSION_PATH", base_prefix.rstrip("/")) + + for subdir in subdirs: + escaped = re.escape(subdir) + text = re.sub( + r"(^|[\s`\"'(])(?:\.?/)?" + escaped + r"/", + r"\1" + base_prefix + subdir + "/", + text, + ) + + return text + def render_markdown_command( self, frontmatter: dict, body: str, source_id: str, context_note: str = None ) -> str: @@ -257,6 +330,62 @@ def render_yaml_command( description = str(description) if description is not None else "" return YamlIntegration._render_yaml(title, description, body, source_id) + def render_agent_definition( + self, + agent_name: str, + skill_name: str, + frontmatter: dict, + body: str, + source_id: str, + source_file: str, + project_root: Path, + source_dir: Optional[Path] = None, + ) -> str: + """Render a command as a Claude agent definition file (.claude/agents/{name}.md). + + Agent definitions differ from skills: + - Body is the system prompt, not a task prompt + - Frontmatter is minimal: name, description, and behavior-derived fields + - No user-invocable, disable-model-invocation, context, or metadata keys + """ + if not isinstance(frontmatter, dict): + frontmatter = {} + + if source_dir is not None and (source_dir / "extension.yml").exists(): + body = self.rewrite_extension_paths(body, source_id, source_dir) + + behavior = frontmatter.get("behavior") or {} + agents_overrides = frontmatter.get("agents") or {} + behavior_fields: dict = {} + if isinstance(behavior, dict): + behavior_fields = translate_behavior( + agent_name, behavior, + agents_overrides if isinstance(agents_overrides, dict) else {} + ) + + clean_frontmatter = strip_behavior_keys(frontmatter) + description = clean_frontmatter.get("description", f"Spec-kit agent: {skill_name}") + + # Agent definition frontmatter: minimal set, no skill-specific keys + agent_fm: dict = { + "name": skill_name, + "description": description, + } + + # Merge behavior-translated fields; remap allowed-tools → tools for agent defs + for k, v in behavior_fields.items(): + if k == "allowed-tools": + agent_fm["tools"] = v + elif k not in {"disable-model-invocation", "user-invocable", "context", "agent"}: + agent_fm[k] = v + + # Explicit model/tools in source frontmatter win + for key in ("model", "tools"): + if key in clean_frontmatter: + agent_fm[key] = clean_frontmatter[key] + + return self.render_frontmatter(agent_fm) + "\n" + body + def render_skill_command( self, agent_name: str, @@ -266,6 +395,7 @@ def render_skill_command( source_id: str, source_file: str, project_root: Path, + source_dir: Optional[Path] = None, ) -> str: """Render a command override as a SKILL.md file. @@ -282,21 +412,47 @@ def render_skill_command( if not isinstance(frontmatter, dict): frontmatter = {} + if source_dir is not None and (source_dir / "extension.yml").exists(): + body = self.rewrite_extension_paths(body, source_id, source_dir) + agent_config = self.AGENT_CONFIGS.get(agent_name, {}) if agent_config.get("extension") == "/SKILL.md": body = self.resolve_skill_placeholders( agent_name, frontmatter, body, project_root ) - description = frontmatter.get( - "description", f"Spec-kit workflow command: {skill_name}" - ) + # Extract and translate behavior + agents escape hatch + behavior = frontmatter.get("behavior") or {} + agents_overrides = frontmatter.get("agents") or {} + behavior_fields: dict = {} + if isinstance(behavior, dict): + behavior_fields = translate_behavior( + agent_name, behavior, + agents_overrides if isinstance(agents_overrides, dict) else {} + ) + + # Strip behavior/agents keys before building skill frontmatter + clean_frontmatter = strip_behavior_keys(frontmatter) + + description = clean_frontmatter.get("description", f"Spec-kit workflow command: {skill_name}") skill_frontmatter = self.build_skill_frontmatter( agent_name, skill_name, description, f"{source_id}:{source_file}", + source_frontmatter=clean_frontmatter, ) + # Merge behavior translation — passthrough (already in skill_frontmatter) wins + # because we only set behavior fields if they are not already set via passthrough, + # EXCEPT for the set of fields that behavior can legitimately override defaults for. + # Only the two boolean defaults injected by build_skill_frontmatter may be + # overridden by behavior translation. Content fields (model, effort, context, + # agent, allowed-tools) set explicitly in source frontmatter must win. + _behavior_overridable = {"disable-model-invocation", "user-invocable"} + for k, v in behavior_fields.items(): + if k not in skill_frontmatter or k in _behavior_overridable: + skill_frontmatter[k] = v + return self.render_frontmatter(skill_frontmatter) + "\n" + body @staticmethod @@ -305,9 +461,22 @@ def build_skill_frontmatter( skill_name: str, description: str, source: str, + source_frontmatter: dict | None = None, ) -> dict: - """Build consistent SKILL.md frontmatter across all skill generators.""" - skill_frontmatter = { + """Build consistent SKILL.md frontmatter across all skill generators. + + Args: + agent_name: Target agent key (e.g. "claude", "codex"). + skill_name: Generated skill name (e.g. "speckit-revenge-extract"). + description: Human-readable description. + source: Source tracking string (e.g. "revenge:commands/extract.md"). + source_frontmatter: Original command frontmatter. Keys present in + ``_SKILL_PASSTHROUGH_KEYS[agent_name]`` are merged after + defaults, allowing source authors to override injected values. + """ + source_frontmatter = source_frontmatter or {} + + skill_frontmatter: dict = { "name": skill_name, "description": description, "compatibility": "Requires spec-kit project structure with .specify/ directory", @@ -316,6 +485,11 @@ def build_skill_frontmatter( "source": source, }, } + + for key in _SKILL_PASSTHROUGH_KEYS.get(agent_name, frozenset()): + if key in source_frontmatter: + skill_frontmatter[key] = source_frontmatter[key] + return skill_frontmatter @staticmethod @@ -469,6 +643,18 @@ def register_commands( content = source_file.read_text(encoding="utf-8") frontmatter, body = self.parse_frontmatter(content) + for key in ("description", "agents"): + if key in cmd_info and key not in frontmatter: + frontmatter[key] = cmd_info[key] + if "behavior" in cmd_info: + manifest_behavior = cmd_info["behavior"] + if isinstance(manifest_behavior, dict): + source_behavior = frontmatter.get("behavior") + if isinstance(source_behavior, dict): + frontmatter["behavior"] = {**manifest_behavior, **source_behavior} + else: + frontmatter["behavior"] = manifest_behavior + if frontmatter.get("strategy") == "wrap": from .presets import _substitute_core_template body, core_frontmatter = _substitute_core_template(body, cmd_name, project_root, self) @@ -494,6 +680,33 @@ def register_commands( output_name = self._compute_output_name(agent_name, cmd_name, agent_config) + # Deployment target is fully derived from behavior.execution + cmd_type = get_deployment_type(frontmatter) + + if cmd_type == "agent" and agent_name == "claude": + output = self.render_agent_definition( + agent_name, output_name, frontmatter, body, + source_id, cmd_file, project_root, source_dir=source_dir, + ) + agents_dir = project_root / ".claude" / "agents" + agents_dir.mkdir(parents=True, exist_ok=True) + dest_file = agents_dir / f"{output_name}.md" + dest_file.write_text(output, encoding="utf-8") + registered.append(cmd_name) + # Also generate agent definition files for any aliases + for alias in cmd_info.get("aliases", []): + alias_output_name = self._compute_output_name(agent_name, alias, agent_config) + alias_frontmatter = deepcopy(frontmatter) + alias_frontmatter["name"] = alias_output_name + alias_output = self.render_agent_definition( + agent_name, alias_output_name, alias_frontmatter, body, + source_id, cmd_file, project_root, source_dir=source_dir, + ) + alias_file = agents_dir / f"{alias_output_name}.md" + alias_file.write_text(alias_output, encoding="utf-8") + registered.append(alias) + continue # skip normal skill/command rendering + if agent_config["extension"] == "/SKILL.md": output = self.render_skill_command( agent_name, @@ -503,11 +716,31 @@ def register_commands( source_id, cmd_file, project_root, + source_dir=source_dir, ) elif agent_config["format"] == "markdown": body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root) body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"]) - output = self.render_markdown_command(frontmatter, body, source_id, context_note) + behavior_raw = frontmatter.get("behavior") + behavior = behavior_raw if isinstance(behavior_raw, dict) else {} + agents_overrides = frontmatter.get("agents") or {} + has_copilot_override = ( + isinstance(agents_overrides, dict) + and isinstance(agents_overrides.get("copilot"), dict) + ) + if agent_name == "copilot" and (isinstance(behavior_raw, dict) or has_copilot_override): + extra_fields = translate_behavior( + agent_name, behavior, + agents_overrides if isinstance(agents_overrides, dict) else {} + ) + copilot_tools = get_copilot_tools(behavior) + if copilot_tools: + extra_fields["tools"] = copilot_tools + copilot_fm = strip_behavior_keys(frontmatter) + copilot_fm.update(extra_fields) + output = self.render_markdown_command(copilot_fm, body, source_id, context_note) + else: + output = self.render_markdown_command(frontmatter, body, source_id, context_note) elif agent_config["format"] == "toml": body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root) body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"]) @@ -552,11 +785,10 @@ def register_commands( source_id, cmd_file, project_root, + source_dir=source_dir, ) elif agent_config["format"] == "markdown": - alias_output = self.render_markdown_command( - alias_frontmatter, body, source_id, context_note - ) + alias_output = self.render_markdown_command(alias_frontmatter, body, source_id, context_note) elif agent_config["format"] == "toml": alias_output = self.render_toml_command( alias_frontmatter, body, source_id @@ -581,6 +813,7 @@ def register_commands( source_id, cmd_file, project_root, + source_dir=source_dir, ) alias_file = ( @@ -736,6 +969,12 @@ def unregister_commands( if prompt_file.exists(): prompt_file.unlink() + # Also try agent definition file (Claude-specific) + if agent_name == "claude": + agent_def = project_root / ".claude" / "agents" / f"{output_name}.md" + if agent_def.exists(): + agent_def.unlink() + # Populate AGENT_CONFIGS after class definition. # Catches ImportError from circular imports during module loading; diff --git a/src/specify_cli/behavior.py b/src/specify_cli/behavior.py new file mode 100644 index 0000000000..3bb230c236 --- /dev/null +++ b/src/specify_cli/behavior.py @@ -0,0 +1,197 @@ +"""Neutral behavior vocabulary for extension commands. + +Extension command source files can declare a ``behavior:`` block in their +frontmatter to express agent-neutral intent (isolation, capability, tools, +etc.). This module translates that vocabulary to concrete per-agent +frontmatter fields during rendering. + +Extension authors can also declare an ``agents:`` escape-hatch block for +agent-specific fields that have no neutral equivalent:: + + behavior: + execution: isolated + capability: strong + effort: high + tools: read-only + invocation: explicit + visibility: user + + agents: + claude: + paths: "src/**" + argument-hint: "Codebase path to analyze" + copilot: + handoffs: + - label: "Generate plan" + agent: speckit.plan + send: true +""" + +from __future__ import annotations + +from copy import deepcopy + +# Keys that belong to the neutral behavior vocabulary +BEHAVIOR_KEYS: frozenset[str] = frozenset({ + "execution", # command | isolated | agent + "capability", # fast | balanced | strong + "effort", # low | medium | high | max + "tools", # none | read-only | write | full | custom list (str or list[str]) + "invocation", # explicit | automatic + "visibility", # user | model | both + "color", # red | blue | green | yellow | purple | orange | pink | cyan (Claude Code UI color) +}) + +# Per-agent translation tables. +# Structure: agent_name -> behavior_key -> value -> (frontmatter_key, frontmatter_value) +# (None, None) means "no frontmatter injection for this combination" +_TRANSLATIONS: dict[str, dict[str, dict[str, tuple[str | None, object]]]] = { + "claude": { + "execution": { + "isolated": ("context", "fork"), + "command": (None, None), + "agent": (None, None), # routing concern, not frontmatter + }, + "capability": { + "fast": ("model", "claude-haiku-4-5-20251001"), + "balanced": ("model", "claude-sonnet-4-6"), + "strong": ("model", "claude-opus-4-6"), + }, + "effort": { + "low": ("effort", "low"), + "medium": ("effort", "medium"), + "high": ("effort", "high"), + "max": ("effort", "max"), + }, + "tools": { + "none": ("allowed-tools", ""), + "read-only": ("allowed-tools", "Read Grep Glob"), + "write": ("allowed-tools", "Read Write Edit Grep Glob"), + "full": (None, None), + }, + "invocation": { + "explicit": ("disable-model-invocation", True), + "automatic": ("disable-model-invocation", False), + }, + "visibility": { + "user": ("user-invocable", True), + "model": ("user-invocable", False), + "both": (None, None), + }, + }, + "copilot": { + "execution": { + "agent": ("mode", "agent"), + "isolated": ("mode", "agent"), + "command": (None, None), + }, + "capability": { + "fast": ("model", "Claude Haiku 4.5"), + "balanced": ("model", "Claude Sonnet 4.5"), + "strong": ("model", "Claude Opus 4.5"), + }, + "invocation": { + "explicit": ("disable-model-invocation", True), + "automatic": ("disable-model-invocation", False), + }, + "visibility": { + "user": ("user-invocable", True), + "model": ("user-invocable", False), + "both": (None, None), + }, + "tools": { + "none": ("tools", []), + "read-only": ("tools", ["read_file", "list_directory", "search_files"]), + "write": ("tools", ["*"]), + "full": ("tools", ["*"]), + }, + }, + "codex": { + "effort": { + "low": ("effort", "low"), + "medium": ("effort", "medium"), + "high": ("effort", "high"), + "max": ("effort", "max"), + }, + }, +} + +# Tools list for Copilot when behavior.tools is set on an agent-type command. +_COPILOT_TOOLS: dict[str, list[str]] = { + "read-only": ["read_file", "list_directory", "search_files"], + "full": [], + "none": [], +} + + +def translate_behavior( + agent_name: str, + behavior: dict, + agents_overrides: dict | None = None, +) -> dict: + """Translate neutral behavior dict to agent-specific frontmatter fields.""" + result: dict = {} + agent_table = _TRANSLATIONS.get(agent_name, {}) + + for key, value in behavior.items(): + if key not in BEHAVIOR_KEYS: + continue + + # color: pass through directly to Claude Code agent frontmatter. + # Valid values: red | blue | green | yellow | purple | orange | pink | cyan + if key == "color" and agent_name == "claude": + result["color"] = str(value) + continue + + # tools: accept a list or a space-separated string of tool names as a + # custom literal, bypassing the preset lookup entirely. + if key == "tools" and agent_name == "claude": + if isinstance(value, list): + result["allowed-tools"] = " ".join(str(t) for t in value) + continue + preset = _TRANSLATIONS.get(agent_name, {}).get("tools", {}).get(str(value)) + if preset is None: + # Unrecognised preset — treat as a literal tool list string + result["allowed-tools"] = str(value) + continue + fm_key, fm_value = preset + if fm_key is not None: + result[fm_key] = fm_value + continue + + key_table = agent_table.get(key, {}) + fm_key, fm_value = key_table.get(str(value), (None, None)) + if fm_key is not None: + result[fm_key] = fm_value + + if agents_overrides and isinstance(agents_overrides, dict): + overrides = agents_overrides.get(agent_name) + if isinstance(overrides, dict): + result.update(overrides) + + return result + + +def get_copilot_tools(behavior: dict) -> list[str]: + """Return Copilot tool list for a given behavior.tools value.""" + tools_value = behavior.get("tools", "full") + return _COPILOT_TOOLS.get(str(tools_value), []) + + +def strip_behavior_keys(frontmatter: dict) -> dict: + """Return a copy of frontmatter with ``behavior:`` and ``agents:`` removed.""" + result = deepcopy(frontmatter) + result.pop("behavior", None) + result.pop("agents", None) + return result + + +def get_deployment_type(frontmatter: dict) -> str: + """Determine deployment type from behavior.execution. + + Returns 'agent' if behavior.execution == 'agent', otherwise 'command'. + """ + behavior = frontmatter.get("behavior") + if isinstance(behavior, dict) and behavior.get("execution") == "agent": + return "agent" + return "command" diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 761b7f31e7..36ee390786 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -22,6 +22,11 @@ import pathspec import yaml +from specify_cli.behavior import ( + get_deployment_type as _get_deployment_type, + strip_behavior_keys, + translate_behavior, +) from packaging import version as pkg_version from packaging.specifiers import SpecifierSet, InvalidSpecifier @@ -907,23 +912,43 @@ def _register_extension_skills( # Do not overwrite user-customized skills continue - # Create skill directory; track whether we created it so we can clean - # up safely if reading the source file subsequently fails. - created_now = not skill_subdir.exists() - skill_subdir.mkdir(parents=True, exist_ok=True) - - # Parse the command file — guard against IsADirectoryError / decode errors + # Read the source file once; reuse for both the deployment-type check + # below and the subsequent skill rendering. try: content = source_file.read_text(encoding="utf-8") except (OSError, UnicodeDecodeError): - if created_now: - try: - skill_subdir.rmdir() # undo the mkdir; dir is empty at this point - except OSError: - pass # best-effort cleanup continue + + # Skip commands that behavior-routing deploys as agent definitions + # (to .claude/agents/) rather than as skill files. + _source_fm, _ = registrar.parse_frontmatter(content) + # Merge manifest-level behavior when source file has none or has an + # invalid (non-dict / empty) value. A non-dict value (string, list, + # null) cannot carry execution:agent, so the manifest entry must win + # for the agent-deployment skip to work correctly. + _source_behavior = _source_fm.get("behavior") + if (not isinstance(_source_behavior, dict) or not _source_behavior) and "behavior" in cmd_info: + _source_fm["behavior"] = cmd_info["behavior"] + if _get_deployment_type(_source_fm) == "agent": + continue + + skill_subdir.mkdir(parents=True, exist_ok=True) frontmatter, body = registrar.parse_frontmatter(content) + + for key in ("agents",): + if key in cmd_info and key not in frontmatter: + frontmatter[key] = cmd_info[key] + if "behavior" in cmd_info: + manifest_behavior = cmd_info["behavior"] + if isinstance(manifest_behavior, dict): + source_behavior = frontmatter.get("behavior") + if isinstance(source_behavior, dict): + frontmatter["behavior"] = {**manifest_behavior, **source_behavior} + else: + frontmatter["behavior"] = manifest_behavior + frontmatter = registrar._adjust_script_paths(frontmatter) + body = registrar.rewrite_extension_paths(body, manifest.id, extension_dir) body = registrar.resolve_skill_placeholders( selected_ai, frontmatter, body, self.project_root ) @@ -931,12 +956,27 @@ def _register_extension_skills( original_desc = frontmatter.get("description", "") description = original_desc or f"Extension command: {cmd_name}" + clean_frontmatter = strip_behavior_keys(frontmatter) frontmatter_data = registrar.build_skill_frontmatter( selected_ai, skill_name, description, f"extension:{manifest.id}", + source_frontmatter=clean_frontmatter, + ) + + behavior = frontmatter.get("behavior") if isinstance(frontmatter.get("behavior"), dict) else {} + agents_overrides = frontmatter.get("agents") or {} + behavior_fields = translate_behavior( + selected_ai, + behavior, + agents_overrides if isinstance(agents_overrides, dict) else {}, ) + overridable_defaults = {"disable-model-invocation", "user-invocable"} + for key, value in behavior_fields.items(): + if key not in frontmatter_data or key in overridable_defaults: + frontmatter_data[key] = value + frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() # Derive a human-friendly title from the command name @@ -1311,11 +1351,12 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: backup_dir = self.extensions_dir / ".backup" / extension_id backup_dir.mkdir(parents=True, exist_ok=True) - # Backup both primary and local override config files - config_files = list(extension_dir.glob("*-config.yml")) + list( - extension_dir.glob("*-config.local.yml") - ) - for config_file in config_files: + # Backup all top-level .yml files except extension.yml (the manifest). + # This preserves user configs (*-config.yml) and any runtime-generated + # state files that extensions write to their own directory. + for config_file in extension_dir.glob("*.yml"): + if config_file.name == "extension.yml": + continue backup_path = backup_dir / config_file.name shutil.copy2(config_file, backup_path) diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index f3b74b0c05..9344131995 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -1460,6 +1460,45 @@ def _quote(v: str) -> str: escaped = v.replace("\\", "\\\\").replace('"', '\\"') return f'"{escaped}"' + # Translate behavior block to agent-specific frontmatter fields. + # This lets templates declare e.g. `behavior: invocation: automatic` + # to produce `disable-model-invocation: false` in the skill. + # Fields are emitted here so downstream post-processors (e.g. + # ClaudeIntegration.setup) see them already set and skip injection. + behavior = frontmatter.get("behavior") + behavior_dict = behavior if isinstance(behavior, dict) else {} + agents_overrides = frontmatter.get("agents") or {} + has_agent_override = ( + isinstance(agents_overrides, dict) + and isinstance(agents_overrides.get(self.key), dict) + ) + behavior_fm_lines = "" + if behavior_dict or has_agent_override: + try: + from specify_cli.behavior import translate_behavior + + behavior_fields = translate_behavior( + self.key, behavior_dict, + agents_overrides if isinstance(agents_overrides, dict) else {} + ) + for bk, bv in behavior_fields.items(): + if isinstance(bv, bool): + behavior_fm_lines += f"{bk}: {'true' if bv else 'false'}\n" + elif isinstance(bv, str): + behavior_fm_lines += f"{bk}: {_quote(bv)}\n" + elif isinstance(bv, (list, dict)): + dumped = yaml.safe_dump( + {bk: bv}, + sort_keys=False, + allow_unicode=True, + default_flow_style=False, + ).rstrip() + behavior_fm_lines += dumped + "\n" + else: + behavior_fm_lines += f"{bk}: {bv}\n" + except ImportError: + pass + skill_content = ( f"---\n" f"name: {_quote(skill_name)}\n" @@ -1468,6 +1507,7 @@ def _quote(v: str) -> str: f"metadata:\n" f" author: {_quote('github-spec-kit')}\n" f" source: {_quote('templates/commands/' + src_file.name)}\n" + f"{behavior_fm_lines}" f"---\n" f"{processed_body}" ) diff --git a/src/specify_cli/integrations/claude/__init__.py b/src/specify_cli/integrations/claude/__init__.py index 3e39db717e..948d45a841 100644 --- a/src/specify_cli/integrations/claude/__init__.py +++ b/src/specify_cli/integrations/claude/__init__.py @@ -115,10 +115,10 @@ def _render_skill(self, template_name: str, frontmatter: dict[str, Any], body: s frontmatter_text = yaml.safe_dump(skill_frontmatter, sort_keys=False).strip() return f"---\n{frontmatter_text}\n---\n\n{body.strip()}\n" - def _build_skill_fm(self, name: str, description: str, source: str) -> dict: + def _build_skill_fm(self, name: str, description: str, source: str, source_frontmatter: dict | None = None) -> dict: from specify_cli.agents import CommandRegistrar return CommandRegistrar.build_skill_frontmatter( - self.key, name, description, source + self.key, name, description, source, source_frontmatter=source_frontmatter ) @staticmethod diff --git a/templates/commands/analyze.md b/templates/commands/analyze.md index 5b521cf2a4..019de83ab9 100644 --- a/templates/commands/analyze.md +++ b/templates/commands/analyze.md @@ -1,5 +1,7 @@ --- description: Perform a non-destructive cross-artifact consistency and quality analysis across spec.md, plan.md, and tasks.md after task generation. +behavior: + invocation: automatic scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks ps: scripts/powershell/check-prerequisites.ps1 -Json -RequireTasks -IncludeTasks diff --git a/templates/commands/checklist.md b/templates/commands/checklist.md index 3e400d86e3..3a3c2293db 100644 --- a/templates/commands/checklist.md +++ b/templates/commands/checklist.md @@ -1,5 +1,7 @@ --- description: Generate a custom checklist for the current feature based on user requirements. +behavior: + invocation: automatic scripts: sh: scripts/bash/check-prerequisites.sh --json ps: scripts/powershell/check-prerequisites.ps1 -Json diff --git a/templates/commands/clarify.md b/templates/commands/clarify.md index 57cd01def1..146cd22bfe 100644 --- a/templates/commands/clarify.md +++ b/templates/commands/clarify.md @@ -1,5 +1,7 @@ --- description: Identify underspecified areas in the current feature spec by asking up to 5 highly targeted clarification questions and encoding answers back into the spec. +behavior: + invocation: automatic handoffs: - label: Build Technical Plan agent: speckit.plan diff --git a/templates/commands/constitution.md b/templates/commands/constitution.md index 29ae9a09e2..2a1cf5d021 100644 --- a/templates/commands/constitution.md +++ b/templates/commands/constitution.md @@ -1,5 +1,7 @@ --- description: Create or update the project constitution from interactive or provided principle inputs, ensuring all dependent templates stay in sync. +behavior: + invocation: automatic handoffs: - label: Build Specification agent: speckit.specify diff --git a/templates/commands/implement.md b/templates/commands/implement.md index 7ba5ba8e0c..0eeea70a03 100644 --- a/templates/commands/implement.md +++ b/templates/commands/implement.md @@ -1,5 +1,7 @@ --- description: Execute the implementation plan by processing and executing all tasks defined in tasks.md +behavior: + invocation: automatic scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks ps: scripts/powershell/check-prerequisites.ps1 -Json -RequireTasks -IncludeTasks diff --git a/templates/commands/plan.md b/templates/commands/plan.md index 04db94ffaa..7c12a6f974 100644 --- a/templates/commands/plan.md +++ b/templates/commands/plan.md @@ -1,5 +1,7 @@ --- description: Execute the implementation planning workflow using the plan template to generate design artifacts. +behavior: + invocation: automatic handoffs: - label: Create Tasks agent: speckit.tasks diff --git a/templates/commands/specify.md b/templates/commands/specify.md index 1f3f5c4465..e3a02f8660 100644 --- a/templates/commands/specify.md +++ b/templates/commands/specify.md @@ -1,5 +1,7 @@ --- description: Create or update the feature specification from a natural language feature description. +behavior: + invocation: automatic handoffs: - label: Build Technical Plan agent: speckit.plan diff --git a/templates/commands/tasks.md b/templates/commands/tasks.md index 4e204abc1b..676d235e8a 100644 --- a/templates/commands/tasks.md +++ b/templates/commands/tasks.md @@ -1,5 +1,7 @@ --- description: Generate an actionable, dependency-ordered tasks.md for the feature based on available design artifacts. +behavior: + invocation: automatic handoffs: - label: Analyze For Consistency agent: speckit.analyze diff --git a/templates/commands/taskstoissues.md b/templates/commands/taskstoissues.md index 77db7be130..9ac09f5c40 100644 --- a/templates/commands/taskstoissues.md +++ b/templates/commands/taskstoissues.md @@ -1,5 +1,7 @@ --- description: Convert existing tasks into actionable, dependency-ordered GitHub issues for the feature based on available design artifacts. +behavior: + invocation: automatic tools: ['github/github-mcp-server/issue_write'] scripts: sh: scripts/bash/check-prerequisites.sh --json --require-tasks --include-tasks diff --git a/tests/integrations/test_integration_claude.py b/tests/integrations/test_integration_claude.py index b3236a66b7..77f52e5326 100644 --- a/tests/integrations/test_integration_claude.py +++ b/tests/integrations/test_integration_claude.py @@ -3,6 +3,7 @@ import codecs import json import os +from textwrap import dedent from unittest.mock import patch import yaml diff --git a/tests/test_agent_deployment.py b/tests/test_agent_deployment.py new file mode 100644 index 0000000000..3f2899cbd8 --- /dev/null +++ b/tests/test_agent_deployment.py @@ -0,0 +1,587 @@ +"""Tests for behavior.execution:agent deployment to agent-specific directories.""" +import json +import yaml +import pytest +import tempfile +import shutil +from pathlib import Path +from textwrap import dedent + +from specify_cli.agents import CommandRegistrar + + +@pytest.fixture +def project_root(tmp_path): + root = tmp_path / "proj" + (root / ".claude" / "skills").mkdir(parents=True) + (root / ".claude" / "agents").mkdir(parents=True) + (root / ".specify").mkdir() + (root / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "claude", "ai_skills": True, "script": "sh"}) + ) + return root + + +@pytest.fixture +def source_dir(tmp_path): + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + return src + + +class TestClaudeAgentDeployment: + def _write_command(self, source_dir, filename, content): + f = source_dir / filename + f.write_text(content) + return f + + def test_no_execution_behavior_deploys_to_skills(self, project_root, source_dir): + self._write_command(source_dir, "hello.md", dedent("""\ + --- + description: Test command + --- + Hello world + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{"name": "speckit.test-ext.hello", "file": "hello.md"}], + "test-ext", source_dir, project_root, + ) + skill_file = project_root / ".claude" / "skills" / "speckit-test-ext-hello" / "SKILL.md" + agent_file = project_root / ".claude" / "agents" / "speckit-test-ext-hello.md" + assert skill_file.exists() + assert not agent_file.exists() + + def test_execution_agent_deploys_to_agents_dir(self, project_root, source_dir): + self._write_command(source_dir, "analyzer.md", dedent("""\ + --- + description: Analyze the codebase + behavior: + execution: agent + capability: strong + tools: read-only + --- + You are a codebase analysis specialist. $ARGUMENTS + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{"name": "speckit.test-ext.analyzer", "file": "analyzer.md"}], + "test-ext", source_dir, project_root, + ) + agent_file = project_root / ".claude" / "agents" / "speckit-test-ext-analyzer.md" + skill_file = project_root / ".claude" / "skills" / "speckit-test-ext-analyzer" / "SKILL.md" + assert agent_file.exists() + assert not skill_file.exists() + + def test_agent_file_has_correct_frontmatter(self, project_root, source_dir): + self._write_command(source_dir, "analyzer.md", dedent("""\ + --- + description: Analyze the codebase + behavior: + execution: agent + capability: strong + tools: read-only + --- + You are a specialist. $ARGUMENTS + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{"name": "speckit.test-ext.analyzer", "file": "analyzer.md"}], + "test-ext", source_dir, project_root, + ) + content = (project_root / ".claude" / "agents" / "speckit-test-ext-analyzer.md").read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) + assert fm["name"] == "speckit-test-ext-analyzer" + assert fm["description"] == "Analyze the codebase" + assert fm.get("model") == "claude-opus-4-6" # from capability: strong + assert fm.get("tools") == "Read Grep Glob" # from tools: read-only + # These must NOT appear in agent definition files + assert "user-invocable" not in fm + assert "disable-model-invocation" not in fm + assert "context" not in fm + assert "behavior" not in fm + + def test_agent_file_body_is_system_prompt(self, project_root, source_dir): + self._write_command(source_dir, "analyzer.md", dedent("""\ + --- + description: Analyze the codebase + behavior: + execution: agent + --- + You are a specialist. $ARGUMENTS + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{"name": "speckit.test-ext.analyzer", "file": "analyzer.md"}], + "test-ext", source_dir, project_root, + ) + content = (project_root / ".claude" / "agents" / "speckit-test-ext-analyzer.md").read_text() + body = "---".join(content.split("---")[2:]).strip() + assert "You are a specialist" in body + + def test_execution_isolated_deploys_to_skills_not_agents(self, project_root, source_dir): + self._write_command(source_dir, "hello.md", dedent("""\ + --- + description: Test isolated + behavior: + execution: isolated + --- + Hello + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{"name": "speckit.test-ext.hello", "file": "hello.md"}], + "test-ext", source_dir, project_root, + ) + skill_file = project_root / ".claude" / "skills" / "speckit-test-ext-hello" / "SKILL.md" + agent_file = project_root / ".claude" / "agents" / "speckit-test-ext-hello.md" + assert skill_file.exists() + assert not agent_file.exists() + + def test_unregister_removes_agent_file(self, project_root, source_dir): + self._write_command(source_dir, "analyzer.md", dedent("""\ + --- + description: Test + behavior: + execution: agent + --- + Body + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{"name": "speckit.test-ext.analyzer", "file": "analyzer.md"}], + "test-ext", source_dir, project_root, + ) + agent_file = project_root / ".claude" / "agents" / "speckit-test-ext-analyzer.md" + assert agent_file.exists() + + registrar.unregister_commands( + {"claude": ["speckit.test-ext.analyzer"]}, + project_root, + ) + assert not agent_file.exists() + + +class TestCopilotAgentDeployment: + """behavior.execution:agent on Copilot injects mode: and tools: into .agent.md frontmatter.""" + + def _setup_copilot_project(self, tmp_path): + root = tmp_path / "proj" + (root / ".github" / "agents").mkdir(parents=True) + (root / ".github" / "prompts").mkdir(parents=True) + (root / ".specify").mkdir() + (root / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "copilot", "script": "sh"}) + ) + return root + + def test_copilot_type_agent_injects_mode(self, tmp_path): + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + (src / "analyzer.md").write_text(dedent("""\ + --- + description: Analyze codebase + behavior: + execution: agent + tools: read-only + --- + Analyze $ARGUMENTS + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.analyzer", "file": "analyzer.md"}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.analyzer.agent.md" + assert agent_file.exists() + content = agent_file.read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) + assert fm.get("mode") == "agent" + assert "read_file" in fm.get("tools", []) + + def test_copilot_type_command_no_tools_injected(self, tmp_path): + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + (src / "hello.md").write_text("---\ndescription: Hello\n---\nHello") + registrar = CommandRegistrar() + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.hello", "file": "hello.md"}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.hello.agent.md" + content = agent_file.read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) or {} + assert "mode" not in fm + assert "tools" not in fm + + def test_copilot_agent_no_tools_key_omits_tools(self, tmp_path): + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + (src / "worker.md").write_text(dedent("""\ + --- + description: Worker agent + behavior: + execution: agent + --- + Do work + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.worker", "file": "worker.md"}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.worker.agent.md" + content = agent_file.read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) or {} + assert fm.get("mode") == "agent" + assert "tools" not in fm + + def test_copilot_agents_override_survives(self, tmp_path): + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + (src / "custom.md").write_text(dedent("""\ + --- + description: Custom agent + behavior: + execution: agent + agents: + copilot: + someCustomKey: someValue + --- + Do custom work + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.custom", "file": "custom.md"}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.custom.agent.md" + content = agent_file.read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) or {} + assert fm.get("someCustomKey") == "someValue" + + def test_copilot_agents_override_without_behavior_survives(self, tmp_path): + """Copilot agents override should apply even when behavior is absent.""" + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + (src / "custom2.md").write_text(dedent("""\ + --- + description: Custom command + agents: + copilot: + someCustomKey: someValue + --- + Do custom work + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.custom2", "file": "custom2.md"}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.custom2.agent.md" + content = agent_file.read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) or {} + assert fm.get("someCustomKey") == "someValue" + assert "agents" not in fm + + def test_copilot_empty_behavior_is_stripped(self, tmp_path): + """Empty behavior mapping should still be stripped from Copilot output.""" + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + (src / "empty-behavior.md").write_text(dedent("""\ + --- + description: Empty behavior + behavior: {} + --- + Do work + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.empty-behavior", "file": "empty-behavior.md"}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.empty-behavior.agent.md" + content = agent_file.read_text() + parts = content.split("---") + fm = yaml.safe_load(parts[1]) or {} + assert "behavior" not in fm + + def test_copilot_non_dict_behavior_does_not_raise(self, tmp_path): + """A non-dict behavior value in Copilot execution:agent branch must not raise.""" + root = self._setup_copilot_project(tmp_path) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + # behavior in source file is a string, not a mapping; manifest provides the execution type + (src / "bad.md").write_text(dedent("""\ + --- + description: Bad behavior + behavior: "this is not a dict" + --- + Body text + """)) + registrar = CommandRegistrar() + # Should not raise even though source behavior is a string; manifest behavior wins + registrar.register_commands( + "copilot", + [{"name": "speckit.test-ext.bad", "file": "bad.md", + "behavior": {"execution": "agent"}}], + "test-ext", src, root, + ) + agent_file = root / ".github" / "agents" / "speckit.test-ext.bad.agent.md" + assert agent_file.exists() + + +class TestEndToEnd: + """Full pipeline: extension with behavior.execution:agent → correct files deployed.""" + + def test_extension_with_agent_command_deploys_correctly(self, tmp_path): + """An extension declaring execution:agent deploys to .claude/agents/, not skills.""" + from specify_cli.extensions import ExtensionManager + + project_root = tmp_path / "proj" + (project_root / ".claude" / "skills").mkdir(parents=True) + (project_root / ".claude" / "agents").mkdir(parents=True) + (project_root / ".specify").mkdir() + # ai_skills is intentionally omitted: skill deployment in this test goes through + # CommandRegistrar.register_commands (which routes based on behavior.execution), + # not the _register_extension_skills path that requires ai_skills to be True. + (project_root / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "claude", "script": "sh"}) + ) + + # Create extension directory with manifest + command + ext_dir = tmp_path / "revenge" + (ext_dir / "commands").mkdir(parents=True) + (ext_dir / "extension.yml").write_text(yaml.dump({ + "schema_version": "1.0", + "extension": { + "id": "revenge", + "name": "Revenge", + "version": "1.0.0", + "description": "Reverse engineering extension", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.revenge.extract", + "file": "commands/extract.md", + "description": "Run extraction pipeline", + }, + { + "name": "speckit.revenge.analyzer", + "file": "commands/analyzer.md", + "description": "Codebase analyzer subagent", + }, + ] + }, + })) + + # Orchestrator command (no execution: → stays a skill) + (ext_dir / "commands" / "extract.md").write_text(dedent("""\ + --- + description: Run extraction pipeline + behavior: + invocation: automatic + --- + Run the extraction pipeline for $ARGUMENTS + """)) + + # Analyzer subagent (execution:agent → .claude/agents/) + (ext_dir / "commands" / "analyzer.md").write_text(dedent("""\ + --- + description: Codebase analyzer subagent + behavior: + execution: agent + capability: strong + tools: read-only + --- + You are a codebase analysis specialist. + Analyze the codebase at $ARGUMENTS and return structured findings. + """)) + + # Install extension + manager = ExtensionManager(project_root) + manager.install_from_directory(ext_dir, speckit_version="0.1.0") + + # extract → .claude/skills/ (no execution: → command type) + skill_file = project_root / ".claude" / "skills" / "speckit-revenge-extract" / "SKILL.md" + assert skill_file.exists(), "extract should deploy as skill" + skill_fm = yaml.safe_load(skill_file.read_text().split("---")[1]) + assert skill_fm.get("disable-model-invocation") is False # behavior: invocation: automatic + + # analyzer → .claude/agents/ (execution:agent) + agent_file = project_root / ".claude" / "agents" / "speckit-revenge-analyzer.md" + assert agent_file.exists(), "analyzer should deploy as agent definition" + agent_fm = yaml.safe_load(agent_file.read_text().split("---")[1]) + assert agent_fm.get("model") == "claude-opus-4-6" # capability: strong + assert agent_fm.get("tools") == "Read Grep Glob" # tools: read-only + assert "user-invocable" not in agent_fm + assert "disable-model-invocation" not in agent_fm + assert "behavior" not in agent_fm + + # analyzer must NOT also be in skills dir + skill_analyzer = project_root / ".claude" / "skills" / "speckit-revenge-analyzer" / "SKILL.md" + assert not skill_analyzer.exists() + + +class TestManifestBehaviorMerge: + """Manifest-level behavior: field is merged into source frontmatter before rendering.""" + + def _setup(self, tmp_path): + root = tmp_path / "proj" + (root / ".claude" / "skills").mkdir(parents=True) + (root / ".claude" / "agents").mkdir(parents=True) + (root / ".specify").mkdir() + (root / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "claude", "ai_skills": True, "script": "sh"}) + ) + src = tmp_path / "ext" / "commands" + src.mkdir(parents=True) + return root, src + + def test_manifest_behavior_merged_when_source_has_no_behavior(self, tmp_path): + """behavior declared in manifest cmd_info reaches the rendered skill when source has none.""" + root, src = self._setup(tmp_path) + # Source file has no behavior block (pure persona prompt with only description) + (src / "agent.md").write_text(dedent("""\ + --- + description: A persona agent + --- + You are a helpful assistant. + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{ + "name": "speckit.test-ext.agent", + "file": "agent.md", + "behavior": {"invocation": "automatic"}, + }], + "test-ext", src, root, + ) + skill_file = root / ".claude" / "skills" / "speckit-test-ext-agent" / "SKILL.md" + assert skill_file.exists() + fm = yaml.safe_load(skill_file.read_text().split("---")[1]) + assert fm.get("disable-model-invocation") is False + + def test_manifest_capability_merged_to_model(self, tmp_path): + """capability in manifest cmd_info produces correct model in the skill.""" + root, src = self._setup(tmp_path) + (src / "cmd.md").write_text("---\ndescription: Strong cmd\n---\nBody") + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{ + "name": "speckit.test-ext.cmd", + "file": "cmd.md", + "behavior": {"capability": "strong"}, + }], + "test-ext", src, root, + ) + skill_file = root / ".claude" / "skills" / "speckit-test-ext-cmd" / "SKILL.md" + fm = yaml.safe_load(skill_file.read_text().split("---")[1]) + assert fm.get("model") == "claude-opus-4-6" + + def test_source_behavior_wins_over_manifest(self, tmp_path): + """When source file declares behavior, it takes precedence over manifest.""" + root, src = self._setup(tmp_path) + (src / "cmd.md").write_text(dedent("""\ + --- + description: Source wins + behavior: + invocation: explicit + --- + Body + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{ + "name": "speckit.test-ext.cmd", + "file": "cmd.md", + # manifest says automatic, but source says explicit — source wins + "behavior": {"invocation": "automatic"}, + }], + "test-ext", src, root, + ) + skill_file = root / ".claude" / "skills" / "speckit-test-ext-cmd" / "SKILL.md" + fm = yaml.safe_load(skill_file.read_text().split("---")[1]) + assert fm.get("disable-model-invocation") is True + + def test_manifest_execution_agent_routes_to_agents_dir(self, tmp_path): + """execution:agent in manifest cmd_info routes a no-frontmatter file to .claude/agents/.""" + root, src = self._setup(tmp_path) + # Pure persona prompt — no frontmatter at all + (src / "persona.md").write_text("You are a specialist agent. $ARGUMENTS") + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{ + "name": "speckit.test-ext.persona", + "file": "persona.md", + "description": "Specialist persona", + "behavior": {"execution": "agent", "capability": "balanced"}, + }], + "test-ext", src, root, + ) + agent_file = root / ".claude" / "agents" / "speckit-test-ext-persona.md" + skill_file = root / ".claude" / "skills" / "speckit-test-ext-persona" / "SKILL.md" + assert agent_file.exists() + assert not skill_file.exists() + fm = yaml.safe_load(agent_file.read_text().split("---")[1]) + assert fm.get("model") == "claude-sonnet-4-6" # capability: balanced + + def test_agent_aliases_generate_agent_files(self, tmp_path): + """Aliases on execution:agent commands get their own .claude/agents/.md files.""" + root, src = self._setup(tmp_path) + (src / "worker.md").write_text(dedent("""\ + --- + description: Worker agent + behavior: + execution: agent + --- + You are a worker agent. + """)) + registrar = CommandRegistrar() + registrar.register_commands( + "claude", + [{ + "name": "speckit.test-ext.worker", + "file": "worker.md", + "aliases": ["speckit.test-ext.w", "speckit.test-ext.work"], + }], + "test-ext", src, root, + ) + agents_dir = root / ".claude" / "agents" + assert (agents_dir / "speckit-test-ext-worker.md").exists() + assert (agents_dir / "speckit-test-ext-w.md").exists() + assert (agents_dir / "speckit-test-ext-work.md").exists() + # Alias file should use the alias name in its frontmatter + fm = yaml.safe_load((agents_dir / "speckit-test-ext-w.md").read_text().split("---")[1]) + assert fm["name"] == "speckit-test-ext-w" + diff --git a/tests/test_behavior_translator.py b/tests/test_behavior_translator.py new file mode 100644 index 0000000000..624193609c --- /dev/null +++ b/tests/test_behavior_translator.py @@ -0,0 +1,215 @@ +# tests/test_behavior_translator.py +import pytest +from specify_cli.behavior import translate_behavior, strip_behavior_keys, get_deployment_type, get_copilot_tools + + +class TestTranslateBehavior: + def test_execution_isolated_claude(self): + result = translate_behavior("claude", {"execution": "isolated"}) + assert result == {"context": "fork"} + + def test_execution_agent_claude_no_frontmatter_key(self): + # 'agent' execution type is handled by routing, not frontmatter + result = translate_behavior("claude", {"execution": "agent"}) + assert "context" not in result + + def test_capability_strong_claude(self): + result = translate_behavior("claude", {"capability": "strong"}) + assert result == {"model": "claude-opus-4-6"} + + def test_capability_fast_claude(self): + result = translate_behavior("claude", {"capability": "fast"}) + assert result == {"model": "claude-haiku-4-5-20251001"} + + def test_effort_high_claude(self): + result = translate_behavior("claude", {"effort": "high"}) + assert result == {"effort": "high"} + + def test_tools_read_only_claude(self): + result = translate_behavior("claude", {"tools": "read-only"}) + assert result == {"allowed-tools": "Read Grep Glob"} + + def test_tools_none_claude(self): + result = translate_behavior("claude", {"tools": "none"}) + assert result == {"allowed-tools": ""} + + def test_tools_full_claude_no_injection(self): + result = translate_behavior("claude", {"tools": "full"}) + assert "allowed-tools" not in result + + def test_invocation_explicit_claude(self): + result = translate_behavior("claude", {"invocation": "explicit"}) + assert result == {"disable-model-invocation": True} + + def test_invocation_automatic_claude(self): + result = translate_behavior("claude", {"invocation": "automatic"}) + assert result == {"disable-model-invocation": False} + + def test_visibility_model_claude(self): + result = translate_behavior("claude", {"visibility": "model"}) + assert result == {"user-invocable": False} + + def test_execution_agent_copilot_injects_mode_agent(self): + result = translate_behavior("copilot", {"execution": "agent"}) + assert result == {"mode": "agent"} + + def test_execution_isolated_copilot_injects_mode_agent(self): + result = translate_behavior("copilot", {"execution": "isolated"}) + assert result == {"mode": "agent"} + + def test_capability_fast_copilot(self): + result = translate_behavior("copilot", {"capability": "fast"}) + assert result == {"model": "Claude Haiku 4.5"} + + def test_capability_balanced_copilot(self): + result = translate_behavior("copilot", {"capability": "balanced"}) + assert result == {"model": "Claude Sonnet 4.5"} + + def test_capability_strong_copilot(self): + result = translate_behavior("copilot", {"capability": "strong"}) + assert result == {"model": "Claude Opus 4.5"} + + def test_invocation_explicit_copilot(self): + result = translate_behavior("copilot", {"invocation": "explicit"}) + assert result == {"disable-model-invocation": True} + + def test_invocation_automatic_copilot(self): + result = translate_behavior("copilot", {"invocation": "automatic"}) + assert result == {"disable-model-invocation": False} + + def test_visibility_user_copilot(self): + result = translate_behavior("copilot", {"visibility": "user"}) + assert result == {"user-invocable": True} + + def test_visibility_model_copilot(self): + result = translate_behavior("copilot", {"visibility": "model"}) + assert result == {"user-invocable": False} + + def test_tools_none_copilot(self): + result = translate_behavior("copilot", {"tools": "none"}) + assert result == {"tools": []} + + def test_tools_read_only_copilot(self): + result = translate_behavior("copilot", {"tools": "read-only"}) + assert result == {"tools": ["read_file", "list_directory", "search_files"]} + + def test_tools_write_copilot(self): + result = translate_behavior("copilot", {"tools": "write"}) + assert result == {"tools": ["*"]} + + def test_tools_full_copilot(self): + result = translate_behavior("copilot", {"tools": "full"}) + assert result == {"tools": ["*"]} + + def test_tools_write_claude(self): + result = translate_behavior("claude", {"tools": "write"}) + assert result == {"allowed-tools": "Read Write Edit Grep Glob"} + + def test_color_passthrough_claude(self): + result = translate_behavior("claude", {"color": "blue"}) + assert result == {"color": "blue"} + + def test_color_any_value_passthrough_claude(self): + for color in ("red", "green", "yellow", "purple", "orange", "pink", "cyan"): + result = translate_behavior("claude", {"color": color}) + assert result == {"color": color} + + def test_color_ignored_for_non_claude_agents(self): + result = translate_behavior("copilot", {"color": "blue"}) + assert "color" not in result + + def test_unknown_key_ignored(self): + result = translate_behavior("claude", {"unknown-key": "value"}) + assert result == {} + + def test_unsupported_agent_returns_empty(self): + result = translate_behavior("gemini", {"execution": "isolated"}) + assert result == {} + + def test_agents_escape_hatch_applied(self): + result = translate_behavior( + "claude", + {"capability": "fast"}, + agents_overrides={"claude": {"model": "claude-opus-4-6", "paths": "src/**"}}, + ) + assert result["model"] == "claude-opus-4-6" + assert result["paths"] == "src/**" + + def test_agents_escape_hatch_other_agent_ignored(self): + result = translate_behavior( + "claude", + {}, + agents_overrides={"codex": {"effort": "high"}}, + ) + assert result == {} + + def test_multiple_behavior_keys(self): + result = translate_behavior("claude", { + "execution": "isolated", + "capability": "strong", + "effort": "max", + "invocation": "explicit", + }) + assert result["context"] == "fork" + assert result["model"] == "claude-opus-4-6" + assert result["effort"] == "max" + assert result["disable-model-invocation"] is True + + +class TestStripBehaviorKeys: + def test_strips_behavior(self): + fm = {"name": "foo", "behavior": {"execution": "isolated"}, "description": "bar"} + result = strip_behavior_keys(fm) + assert "behavior" not in result + assert result["name"] == "foo" + + def test_strips_agents(self): + fm = {"name": "foo", "agents": {"claude": {"paths": "src/**"}}} + result = strip_behavior_keys(fm) + assert "agents" not in result + + def test_no_behavior_keys_passthrough(self): + fm = {"name": "foo", "description": "bar"} + result = strip_behavior_keys(fm) + assert result == {"name": "foo", "description": "bar"} + + def test_returns_copy_not_mutating_original(self): + fm = {"behavior": {"execution": "isolated"}} + result = strip_behavior_keys(fm) + assert "behavior" in fm # original unchanged + + +class TestGetDeploymentType: + def test_behavior_execution_agent(self): + assert get_deployment_type({"behavior": {"execution": "agent"}}) == "agent" + + def test_behavior_execution_isolated_is_command(self): + assert get_deployment_type({"behavior": {"execution": "isolated"}}) == "command" + + def test_behavior_execution_command_is_command(self): + assert get_deployment_type({"behavior": {"execution": "command"}}) == "command" + + def test_defaults_to_command_when_no_behavior(self): + assert get_deployment_type({}) == "command" + + def test_defaults_to_command_when_no_execution(self): + assert get_deployment_type({"behavior": {"capability": "strong"}}) == "command" + + +class TestGetCopilotTools: + def test_read_only_returns_tools(self): + result = get_copilot_tools({"tools": "read-only"}) + assert "read_file" in result + assert "list_directory" in result + + def test_full_returns_empty(self): + result = get_copilot_tools({"tools": "full"}) + assert result == [] + + def test_none_returns_empty(self): + result = get_copilot_tools({"tools": "none"}) + assert result == [] + + def test_missing_tools_defaults_to_full(self): + result = get_copilot_tools({}) + assert result == [] diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index 89e8b4a8b8..81eefb9ab2 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -16,6 +16,7 @@ import shutil import yaml from pathlib import Path +from textwrap import dedent from specify_cli.extensions import ( ExtensionManifest, @@ -735,3 +736,238 @@ def test_remove_cleans_up_when_ai_skills_toggled(self, skills_project, extension assert result is True assert not (skills_dir / "speckit-test-ext-hello").exists() assert not (skills_dir / "speckit-test-ext-world").exists() + + +class TestPassthroughFrontmatter: + """Source frontmatter keys in _SKILL_PASSTHROUGH_KEYS survive into generated SKILL.md.""" + + def _make_project(self, tmp_path): + """Create project root with claude skills dir.""" + project_root = tmp_path / "proj" + (project_root / ".claude" / "skills").mkdir(parents=True) + (project_root / ".specify").mkdir() + (project_root / ".specify" / "init-options.json").write_text( + '{"ai": "claude", "ai_skills": true, "script": "sh"}' + ) + return project_root + + def test_context_fork_passed_through(self, tmp_path): + import yaml + from specify_cli.agents import CommandRegistrar + project_root = self._make_project(tmp_path) + registrar = CommandRegistrar() + result = registrar.render_skill_command( + "claude", "speckit-test-ext-hello", + {"name": "speckit.test-ext.hello", "description": "Test", "context": "fork", "agent": "general-purpose"}, + "Hello world", "test-ext", "commands/hello.md", project_root, + ) + fm_text = result.split("---")[1] + fm = yaml.safe_load(fm_text) + assert fm.get("context") == "fork" + assert fm.get("agent") == "general-purpose" + + def test_disable_model_invocation_override(self, tmp_path): + import yaml + from specify_cli.agents import CommandRegistrar + project_root = self._make_project(tmp_path) + registrar = CommandRegistrar() + result = registrar.render_skill_command( + "claude", "speckit-test-ext-hello", + {"description": "Test", "disable-model-invocation": False}, + "Hello", "test-ext", "commands/hello.md", project_root, + ) + fm_text = result.split("---")[1] + fm = yaml.safe_load(fm_text) + assert fm.get("disable-model-invocation") is False + + def test_non_passthrough_key_not_leaked(self, tmp_path): + import yaml + from specify_cli.agents import CommandRegistrar + project_root = self._make_project(tmp_path) + registrar = CommandRegistrar() + result = registrar.render_skill_command( + "claude", "speckit-test-ext-hello", + {"description": "Test", "scripts": {"sh": "run.sh"}}, + "Hello", "test-ext", "commands/hello.md", project_root, + ) + fm_text = result.split("---")[1] + fm = yaml.safe_load(fm_text) + assert "scripts" not in fm + + +class TestBehaviorTranslationInRender: + """behavior: and agents: blocks are stripped and translated during rendering.""" + + def _render(self, source_frontmatter: dict, body: str = "Hello") -> dict: + import yaml + import json + import tempfile + from specify_cli.agents import CommandRegistrar + with tempfile.TemporaryDirectory() as tmp: + project_root = Path(tmp) + (project_root / ".specify").mkdir() + (project_root / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "claude", "ai_skills": True, "script": "sh"}) + ) + registrar = CommandRegistrar() + result = registrar.render_skill_command( + "claude", "speckit-test-cmd", + source_frontmatter, body, "test-ext", "commands/test.md", project_root, + ) + parts = result.split("---") + return yaml.safe_load(parts[1]) + + def test_behavior_key_stripped_from_output(self): + fm = self._render({"description": "Test", "behavior": {"execution": "isolated"}}) + assert "behavior" not in fm + + def test_agents_key_stripped_from_output(self): + fm = self._render({"description": "Test", "agents": {"claude": {"paths": "src/**"}}}) + assert "agents" not in fm + + def test_execution_isolated_injects_context_fork(self): + fm = self._render({"description": "Test", "behavior": {"execution": "isolated"}}) + assert fm.get("context") == "fork" + + def test_capability_strong_injects_model(self): + fm = self._render({"description": "Test", "behavior": {"capability": "strong"}}) + assert fm.get("model") == "claude-opus-4-6" + + def test_effort_high_injected(self): + fm = self._render({"description": "Test", "behavior": {"effort": "high"}}) + assert fm.get("effort") == "high" + + def test_tools_read_only_injects_allowed_tools(self): + fm = self._render({"description": "Test", "behavior": {"tools": "read-only"}}) + assert fm.get("allowed-tools") == "Read Grep Glob" + + def test_invocation_automatic_overrides_default(self): + fm = self._render({"description": "Test", "behavior": {"invocation": "automatic"}}) + assert fm.get("disable-model-invocation") is False + + def test_agents_escape_hatch_applied(self): + fm = self._render({ + "description": "Test", + "behavior": {"capability": "fast"}, + "agents": {"claude": {"model": "claude-opus-4-6", "paths": "src/**"}}, + }) + assert fm.get("model") == "claude-opus-4-6" + assert fm.get("paths") == "src/**" + + def test_passthrough_wins_over_behavior(self): + # Explicit context: fork in source FM (passthrough) should still work alongside behavior + fm = self._render({ + "description": "Test", + "context": "fork", + "behavior": {"execution": "isolated"}, + }) + assert fm.get("context") == "fork" + + +# ===== Agent-Routing Skip in _register_extension_skills ===== + +class TestExtensionSkillAgentRoutingSkip: + """_register_extension_skills() must not create SKILL.md for execution:agent commands.""" + + def _make_ext(self, temp_dir: Path, ext_id: str, commands: list) -> Path: + ext_dir = temp_dir / ext_id + (ext_dir / "commands").mkdir(parents=True) + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": ext_id, + "name": ext_id, + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": commands}, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + return ext_dir + + def test_agent_command_not_registered_as_skill(self, skills_project, temp_dir): + """Command with behavior: execution: agent must not create a SKILL.md.""" + project_dir, skills_dir = skills_project + ext_dir = self._make_ext(temp_dir, "routing-ext", [ + { + "name": "speckit.routing-ext.orchestrator", + "file": "commands/orchestrator.md", + "description": "Orchestrator command (plain skill)", + }, + { + "name": "speckit.routing-ext.specialist", + "file": "commands/specialist.md", + "description": "Specialist subagent", + }, + ]) + (ext_dir / "commands" / "orchestrator.md").write_text( + "---\ndescription: Orchestrator\nbehavior:\n invocation: automatic\n---\nOrchestrate.\n" + ) + (ext_dir / "commands" / "specialist.md").write_text( + "---\ndescription: Specialist\nbehavior:\n execution: agent\n---\nYou are a specialist.\n" + ) + + manager = ExtensionManager(project_dir) + manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + # orchestrator → SKILL.md created + assert (skills_dir / "speckit-routing-ext-orchestrator" / "SKILL.md").exists() + # specialist → NO SKILL.md (routed to agents dir instead) + assert not (skills_dir / "speckit-routing-ext-specialist" / "SKILL.md").exists() + + metadata = manager.registry.get(manifest.id) + assert "speckit-routing-ext-orchestrator" in metadata["registered_skills"] + assert "speckit-routing-ext-specialist" not in metadata["registered_skills"] + + def test_agent_command_from_manifest_behavior_not_registered_as_skill(self, skills_project, temp_dir): + """execution:agent declared in manifest cmd_info (not source) also skips SKILL.md creation.""" + project_dir, skills_dir = skills_project + ext_dir = self._make_ext(temp_dir, "manifest-routing-ext", [ + { + "name": "speckit.manifest-routing-ext.agent", + "file": "commands/agent.md", + "description": "Agent from manifest behavior", + "behavior": {"execution": "agent"}, + }, + ]) + # Source file has NO frontmatter — pure persona prompt + (ext_dir / "commands" / "agent.md").write_text("You are a helpful agent.\n") + + manager = ExtensionManager(project_dir) + manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + assert not (skills_dir / "speckit-manifest-routing-ext-agent" / "SKILL.md").exists() + metadata = manager.registry.get(manifest.id) + assert "speckit-manifest-routing-ext-agent" not in metadata["registered_skills"] + + def test_extension_skill_body_paths_rewritten(self, skills_project, temp_dir): + """_register_extension_skills rewrites extension-relative paths before placeholder resolution.""" + project_dir, skills_dir = skills_project + ext_dir = self._make_ext(temp_dir, "path-rewrite-ext", [ + { + "name": "speckit.path-rewrite-ext.cmd", + "file": "commands/cmd.md", + "description": "Command with extension-relative paths", + }, + ]) + # Create a subdir so rewrite_extension_paths picks it up + (ext_dir / "agents").mkdir() + (ext_dir / "commands" / "cmd.md").write_text(dedent("""\ + --- + description: Test command + --- + See agents/control/commander.md for instructions. + """)) + + manager = ExtensionManager(project_dir) + manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + skill_file = skills_dir / "speckit-path-rewrite-ext-cmd" / "SKILL.md" + assert skill_file.exists() + content = skill_file.read_text() + # Path must be rewritten to the installed extension location + assert ".specify/extensions/path-rewrite-ext/agents/control/commander.md" in content + # Must NOT appear as a bare relative path without the full prefix + assert "See agents/control/commander.md" not in content diff --git a/tests/test_extensions.py b/tests/test_extensions.py index c5be0ab4f3..147dc99f3c 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1471,6 +1471,289 @@ def test_all_skill_agents_register_commands_with_resolved_placeholders( assert "{ARGS}" not in content, f"{{ARGS}} not resolved for {agent_name}" assert '.specify/scripts/bash/setup-plan.sh' in content + def test_skill_registration_rewrites_extension_relative_paths(self, project_dir, temp_dir): + """Extension subdirectory paths in command bodies should be rewritten to + .specify/extensions//... in generated SKILL.md files.""" + import yaml + + ext_dir = temp_dir / "ext-multidir" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "agents").mkdir() + (ext_dir / "templates").mkdir() + (ext_dir / "scripts").mkdir() + (ext_dir / "knowledge-base").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "ext-multidir", + "name": "Multi-Dir Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.ext-multidir.run", + "file": "commands/run.md", + "description": "Run command", + } + ] + }, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands" / "run.md").write_text( + "---\n" + "description: Run command\n" + "---\n\n" + "Read agents/control/commander.md for instructions.\n" + "Use templates/report.md as output format.\n" + "Run scripts/bash/gate.sh to validate.\n" + "Load knowledge-base/scores.yaml for calibration.\n" + "Also check memory/constitution.md for project rules.\n" + ) + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) + + content = (skills_dir / "speckit-ext-multidir-run" / "SKILL.md").read_text() + # Extension-owned directories → extension-local paths + assert ".specify/extensions/ext-multidir/agents/control/commander.md" in content + assert ".specify/extensions/ext-multidir/templates/report.md" in content + assert ".specify/extensions/ext-multidir/scripts/bash/gate.sh" in content + assert ".specify/extensions/ext-multidir/knowledge-base/scores.yaml" in content + # memory/ is not an extension directory, so stays project-level + assert "memory/constitution.md" in content + # No bare extension-relative path references remain + assert "Read agents/" not in content + assert "Load knowledge-base/" not in content + + def test_skill_registration_rewrites_extension_relative_paths_for_kimi(self, project_dir, temp_dir): + """Path rewriting should also apply to kimi, which uses the /SKILL.md extension.""" + import yaml + + ext_dir = temp_dir / "ext-kimi-paths" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "agents").mkdir() + (ext_dir / "knowledge-base").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "ext-kimi-paths", + "name": "Kimi Paths Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.ext-kimi-paths.run", + "file": "commands/run.md", + "description": "Run command", + } + ] + }, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands" / "run.md").write_text( + "---\n" + "description: Run command\n" + "---\n\n" + "Read agents/control/commander.md for instructions.\n" + "Load knowledge-base/scores.yaml for calibration.\n" + ) + + skills_dir = project_dir / ".kimi" / "skills" + skills_dir.mkdir(parents=True) + + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("kimi", manifest, ext_dir, project_dir) + + content = (skills_dir / "speckit-ext-kimi-paths-run" / "SKILL.md").read_text() + assert ".specify/extensions/ext-kimi-paths/agents/control/commander.md" in content + assert ".specify/extensions/ext-kimi-paths/knowledge-base/scores.yaml" in content + assert "Read agents/" not in content + + def test_skill_registration_rewrites_paths_in_aliases(self, project_dir, temp_dir): + """Alias SKILL.md files should also have extension-relative paths rewritten.""" + import yaml + + ext_dir = temp_dir / "ext-alias-paths" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "agents").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "ext-alias-paths", + "name": "Alias Paths Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.ext-alias-paths.run", + "file": "commands/run.md", + "description": "Run command", + "aliases": ["speckit.ext-alias-paths.go"], + } + ] + }, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands" / "run.md").write_text( + "---\n" + "description: Run command\n" + "---\n\n" + "Read agents/control/commander.md for instructions.\n" + ) + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) + + alias_content = (skills_dir / "speckit-ext-alias-paths-go" / "SKILL.md").read_text() + assert ".specify/extensions/ext-alias-paths/agents/control/commander.md" in alias_content + assert "Read agents/" not in alias_content + + def test_rewrite_extension_paths_specs_not_rewritten(self, project_dir, temp_dir): + """Extension-internal specs/ dir must not cause user project specs/ paths to be rewritten. + + echelon has a specs/ subdirectory for its own internal design documents. + References to specs/ in command bodies should NOT be rewritten to + .specify/extensions/echelon/specs/ — they point to the user's project specs. + """ + import yaml + + ext_dir = temp_dir / "echelon" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "agents").mkdir() + # Simulate echelon having its own internal specs/ subdir + (ext_dir / "specs").mkdir() + (ext_dir / "specs" / "001-internal").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": {"id": "echelon", "name": "Echelon", "version": "1.0.0", "description": "Test"}, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": [{"name": "speckit.echelon.run", "file": "commands/run.md", "description": "Run"}]}, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands" / "run.md").write_text( + "---\ndescription: Run\n---\n\n" + "Artifacts go to specs/{NNN}-{feature}/.\n" + "Read agents/control/commander.md.\n" + ) + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + CommandRegistrar().register_commands_for_agent("codex", manifest, ext_dir, project_dir) + + content = (skills_dir / "speckit-echelon-run" / "SKILL.md").read_text() + # specs/ must NOT be rewritten to extension-internal path + assert ".specify/extensions/echelon/specs/" not in content + assert "specs/{NNN}-{feature}/" in content + # agents/ should still be rewritten (it is extension-internal) + assert ".specify/extensions/echelon/agents/control/commander.md" in content + + def test_rewrite_extension_paths_no_subdirs(self, project_dir, temp_dir): + """Extension with no subdirectories should leave command body text unchanged.""" + import yaml + + ext_dir = temp_dir / "bare-ext" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": {"id": "bare-ext", "name": "Bare", "version": "1.0.0", "description": "Test"}, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": [{"name": "speckit.bare-ext.run", "file": "commands/run.md", "description": "Run"}]}, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands" / "run.md").write_text( + "---\ndescription: Run\n---\n\nRead agents/control/commander.md and templates/report.md.\n" + ) + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + manifest = ExtensionManifest(ext_dir / "extension.yml") + CommandRegistrar().register_commands_for_agent("codex", manifest, ext_dir, project_dir) + + content = (skills_dir / "speckit-bare-ext-run" / "SKILL.md").read_text() + # No subdirs to match — text unchanged + assert "agents/control/commander.md" in content + assert "templates/report.md" in content + + def test_preset_skill_registration_does_not_rewrite_paths(self, project_dir, temp_dir): + """Preset source dirs (no extension.yml) must not have paths rewritten to .specify/extensions/...""" + import yaml + + preset_dir = temp_dir / "my-preset" + preset_dir.mkdir() + (preset_dir / "commands").mkdir() + # Preset dirs may have a templates/ subdir — must not be rewritten. + (preset_dir / "templates").mkdir() + # No extension.yml — this is a preset, not an extension. + (preset_dir / "preset.yml").write_text("id: my-preset\n") + + commands = [ + { + "name": "speckit.my-preset.run", + "file": "commands/run.md", + "description": "Run", + } + ] + (preset_dir / "commands" / "run.md").write_text( + "---\ndescription: Run\n---\n\nSee templates/report.md for output format.\n" + ) + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + + AgentCommandRegistrar().register_commands_for_all_agents( + commands, "my-preset", preset_dir, project_dir + ) + + content = (skills_dir / "speckit-my-preset-run" / "SKILL.md").read_text() + # Paths must NOT be rewritten to extension-style locations. + assert ".specify/extensions/" not in content + # Original reference must remain intact. + assert "templates/report.md" in content + def test_codex_skill_alias_frontmatter_matches_alias_name(self, project_dir, temp_dir): """Codex alias skills should render their own matching `name:` frontmatter.""" import yaml diff --git a/tests/test_integration_extension_skill_paths.py b/tests/test_integration_extension_skill_paths.py new file mode 100644 index 0000000000..0456a3d24a --- /dev/null +++ b/tests/test_integration_extension_skill_paths.py @@ -0,0 +1,214 @@ +""" +Integration tests: install a real extension into a temp project and verify +that generated SKILL.md files have correct .specify/extensions//… paths +instead of bare extension-relative references. + +Set the SPECKIT_TEST_EXT_DIR environment variable to the path of a local +extension checkout before running. Tests are skipped automatically when +the variable is not set or the directory does not exist. + +Example: + SPECKIT_TEST_EXT_DIR=~/work/my-extension pytest tests/test_integration_extension_skill_paths.py +""" + +import json +import os +import re +import shutil +import tempfile +from pathlib import Path + +import pytest + +_ext_dir_env = os.environ.get("SPECKIT_TEST_EXT_DIR", "") +EXT_DIR = Path(_ext_dir_env).expanduser().resolve() if _ext_dir_env else None + +pytestmark = pytest.mark.skipif( + EXT_DIR is None or not EXT_DIR.exists(), + reason="Set SPECKIT_TEST_EXT_DIR to an extension checkout to run these tests", +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _ext_id() -> str: + from specify_cli.extensions import ExtensionManifest + return ExtensionManifest(EXT_DIR / "extension.yml").id + + +def _make_project(tmp: Path, ai: str = "codex") -> Path: + project = tmp / "project" + project.mkdir() + specify = project / ".specify" + specify.mkdir() + (specify / "init-options.json").write_text( + json.dumps({"ai": ai, "ai_skills": True, "script": "sh"}) + ) + if ai == "codex": + (project / ".agents" / "skills").mkdir(parents=True) + elif ai == "kimi": + (project / ".kimi" / "skills").mkdir(parents=True) + return project + + +def _install_ext(project: Path) -> None: + from specify_cli.extensions import ExtensionManager + try: + from importlib.metadata import version + speckit_version = version("specify-cli") + except Exception: + speckit_version = "999.0.0" + ExtensionManager(project).install_from_directory(EXT_DIR, speckit_version, register_commands=True) + + +def _skill_files(project: Path, ext_id: str, ai: str = "codex") -> dict[str, Path]: + skills_root = project / (".agents/skills" if ai == "codex" else ".kimi/skills") + return { + p.parent.name: p + for p in skills_root.glob("*/SKILL.md") + if ext_id in p.parent.name + } + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +@pytest.fixture(scope="module") +def ext_id(): + return _ext_id() + + +@pytest.fixture +def tmp_dir(): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + +@pytest.fixture +def codex_project(tmp_dir): + project = _make_project(tmp_dir, ai="codex") + _install_ext(project) + return project + + +@pytest.fixture +def kimi_project(tmp_dir): + project = _make_project(tmp_dir, ai="kimi") + _install_ext(project) + return project + + +# --------------------------------------------------------------------------- +# Installation sanity +# --------------------------------------------------------------------------- + +class TestExtensionInstallation: + + def test_extension_files_copied_to_specify_dir(self, codex_project, ext_id): + installed = codex_project / ".specify" / "extensions" / ext_id + assert installed.is_dir() + assert (installed / "extension.yml").exists() + + def test_agent_subdirectory_installed(self, codex_project, ext_id): + installed = codex_project / ".specify" / "extensions" / ext_id + subdirs = [d.name for d in installed.iterdir() if d.is_dir()] + assert subdirs, f"No subdirectories found under {installed}" + + def test_all_commands_produce_skill_files(self, codex_project, ext_id): + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest( + codex_project / ".specify" / "extensions" / ext_id / "extension.yml" + ) + skill_files = _skill_files(codex_project, ext_id) + for cmd in manifest.commands: + short = cmd["name"].removeprefix("speckit.").replace(".", "-") + skill_name = f"speckit-{short}" + assert skill_name in skill_files, ( + f"Expected SKILL.md for '{cmd['name']}' at '{skill_name}'.\n" + f"Available: {sorted(skill_files)}" + ) + + def test_registry_records_installed_extension(self, codex_project, ext_id): + from specify_cli.extensions import ExtensionManager + assert ExtensionManager(codex_project).registry.is_installed(ext_id) + + +# --------------------------------------------------------------------------- +# Path rewriting +# --------------------------------------------------------------------------- + +class TestSkillPathRewriting: + + def test_installed_subdirs_appear_with_extension_prefix(self, codex_project, ext_id): + """At least one installed subdirectory should appear prefixed in skill files.""" + installed = codex_project / ".specify" / "extensions" / ext_id + skill_files = _skill_files(codex_project, ext_id) + all_content = "\n".join(p.read_text() for p in skill_files.values()) + + prefix = f".specify/extensions/{ext_id}/" + installed_subdirs = [d.name for d in installed.iterdir() if d.is_dir() and d.name != "commands"] + rewritten = [s for s in installed_subdirs if f"{prefix}{s}/" in all_content] + assert rewritten, ( + f"No installed subdir appeared as {prefix}/ in any skill file.\n" + f"Installed subdirs: {installed_subdirs}" + ) + + def test_no_bare_subdir_paths_remain(self, codex_project, ext_id): + """No bare '/…' references should survive in any skill file.""" + installed = codex_project / ".specify" / "extensions" / ext_id + skill_files = _skill_files(codex_project, ext_id) + prefix = f".specify/extensions/{ext_id}/" + installed_subdirs = [d.name for d in installed.iterdir() if d.is_dir() and d.name != "commands"] + failures = [] + for subdir in installed_subdirs: + for name, path in skill_files.items(): + stripped = path.read_text().replace(f"{prefix}{subdir}/", "__OK__") + bare = re.findall( + r'(?:^|[\s`"\'(])(?:\.?/)?' + re.escape(subdir) + r'/', + stripped, re.MULTILINE, + ) + if bare: + failures.append(f"{name}: bare '{subdir}/': {bare}") + assert not failures, "Bare subdirectory references found:\n" + "\n".join(failures) + + +# --------------------------------------------------------------------------- +# Kimi +# --------------------------------------------------------------------------- + +class TestSkillPathRewritingKimi: + + def test_kimi_skills_contain_extension_prefix(self, kimi_project, ext_id): + installed = kimi_project / ".specify" / "extensions" / ext_id + skill_files = _skill_files(kimi_project, ext_id, ai="kimi") + assert skill_files, f"No kimi skill files found for {ext_id}" + + prefix = f".specify/extensions/{ext_id}/" + installed_subdirs = [d.name for d in installed.iterdir() if d.is_dir() and d.name != "commands"] + all_content = "\n".join(p.read_text() for p in skill_files.values()) + rewritten = [s for s in installed_subdirs if f"{prefix}{s}/" in all_content] + assert rewritten, ( + f"No installed subdir appeared as {prefix}/ in kimi skill files.\n" + f"Installed subdirs: {installed_subdirs}" + ) + + +# --------------------------------------------------------------------------- +# Script placeholders +# --------------------------------------------------------------------------- + +class TestScriptPlaceholders: + + def test_no_unresolved_script_placeholders(self, codex_project, ext_id): + skill_files = _skill_files(codex_project, ext_id) + failures = [] + for name, path in skill_files.items(): + content = path.read_text() + for placeholder in ("{SCRIPT}", "{AGENT_SCRIPT}", "{ARGS}"): + if placeholder in content: + failures.append(f"{name}: contains {placeholder}") + assert not failures, "Unresolved placeholders:\n" + "\n".join(failures) diff --git a/tests/test_pr2103_reviews.py b/tests/test_pr2103_reviews.py new file mode 100644 index 0000000000..46de1fae50 --- /dev/null +++ b/tests/test_pr2103_reviews.py @@ -0,0 +1,873 @@ +"""Tests for issues raised in PR #2103 code review. + +Covers five bugs found by Copilot inline review: + Issue 1 — Copilot execution:isolated never gets mode:agent injected; behavior/agents keys leak + Issue 2 — Non-dict behavior in source file bypasses manifest-level agent-deployment skip + Issue 3 — SkillsIntegration.setup() ignores agents: escape-hatch overrides + Issue 4 — _behavior_overridable clobbers explicit source frontmatter (model, context, etc.) + Issue 5 — extensions.py _register_extension_skills reads source file twice per iteration +""" + +import json +import shutil +import tempfile +from pathlib import Path +from unittest.mock import patch + +import pytest + +from specify_cli.extensions import CommandRegistrar + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_ext_dir(base: Path, ext_id: str, commands: list[dict]) -> Path: + """Create a minimal installed extension directory.""" + import yaml + + ext_dir = base / ext_id + ext_dir.mkdir(parents=True) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": ext_id, + "name": ext_id, + "version": "1.0.0", + "description": "Test extension", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": [c for c in commands]}, + } + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + (ext_dir / "commands").mkdir() + return ext_dir + + +def _make_project(base: Path, ai: str = "codex", ai_skills: bool = False) -> Path: + """Create a minimal spec-kit project directory.""" + proj = base / "project" + proj.mkdir() + (proj / ".specify").mkdir() + + init_opts: dict = {"ai": ai} + if ai_skills: + init_opts["ai_skills"] = True + (proj / ".specify" / "init-options.json").write_text(json.dumps(init_opts)) + + return proj + + +def _parse_frontmatter(text: str) -> dict: + import yaml + + if not text.startswith("---"): + return {} + parts = text.split("---", 2) + if len(parts) < 3: + return {} + return yaml.safe_load(parts[1]) or {} + + +# --------------------------------------------------------------------------- +# Issue 1 — Copilot execution:isolated does not produce mode:agent +# --------------------------------------------------------------------------- + +class TestCopilotExecutionIsolated: + """Issue 1: behavior.execution=isolated must inject mode:agent for Copilot.""" + + @pytest.fixture + def tmp(self): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + def _copilot_ext(self, tmp: Path, body_frontmatter: str) -> tuple[Path, Path]: + ext_dir = _make_ext_dir(tmp, "ext-cop", [ + {"name": "speckit.ext-cop.cmd", "file": "commands/cmd.md"} + ]) + (ext_dir / "commands" / "cmd.md").write_text( + f"{body_frontmatter}\n\nCommand body." + ) + proj = tmp / "proj" + proj.mkdir() + (proj / ".github" / "agents").mkdir(parents=True) + return ext_dir, proj + + def test_execution_isolated_produces_mode_agent(self, tmp): + """execution:isolated must inject mode:agent into Copilot .agent.md.""" + fm = "---\ndescription: Test cmd\nbehavior:\n execution: isolated\n---" + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + agent_file = proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md" + assert agent_file.exists(), "agent file not created" + content = agent_file.read_text() + parsed = _parse_frontmatter(content) + assert parsed.get("mode") == "agent", ( + f"expected mode:agent in frontmatter, got: {parsed}" + ) + + def test_execution_isolated_strips_behavior_key(self, tmp): + """behavior: key must not appear in generated Copilot .agent.md.""" + fm = "---\ndescription: Test cmd\nbehavior:\n execution: isolated\n---" + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + content = (proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md").read_text() + parsed = _parse_frontmatter(content) + assert "behavior" not in parsed, f"behavior: key leaked into output: {parsed}" + + def test_execution_isolated_strips_agents_key(self, tmp): + """agents: key must not appear in generated Copilot .agent.md.""" + fm = ( + "---\ndescription: Test cmd\n" + "behavior:\n execution: isolated\n" + "agents:\n copilot:\n handoffs: []\n---" + ) + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + content = (proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md").read_text() + parsed = _parse_frontmatter(content) + assert "agents" not in parsed, f"agents: key leaked into output: {parsed}" + + def test_execution_agent_still_works(self, tmp): + """Regression: execution:agent must still produce mode:agent.""" + fm = "---\ndescription: Test cmd\nbehavior:\n execution: agent\n---" + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + agent_file = proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md" + assert agent_file.exists() + parsed = _parse_frontmatter(agent_file.read_text()) + assert parsed.get("mode") == "agent" + + def test_no_behavior_dict_uses_raw_frontmatter(self, tmp): + """No behavior → output frontmatter identical to source (minus behavior/agents).""" + fm = "---\ndescription: Plain cmd\nmodel: claude-opus-4-6\n---" + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + agent_file = proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md" + assert agent_file.exists() + parsed = _parse_frontmatter(agent_file.read_text()) + assert "mode" not in parsed, "mode injected without behavior dict" + + def test_agents_override_applied_for_isolated(self, tmp): + """agents: escape-hatch overrides must be applied for execution:isolated.""" + fm = ( + "---\ndescription: Test cmd\n" + "behavior:\n execution: isolated\n" + "agents:\n copilot:\n priority: 5\n---" + ) + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + agent_file = proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md" + content = agent_file.read_text() + parsed = _parse_frontmatter(content) + assert parsed.get("mode") == "agent" + assert parsed.get("priority") == 5, f"agents: override not applied: {parsed}" + + def test_behavior_command_execution_no_mode_injected(self, tmp): + """execution:command must NOT inject mode into Copilot output.""" + fm = "---\ndescription: Test cmd\nbehavior:\n execution: command\n---" + ext_dir, proj = self._copilot_ext(tmp, fm) + + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("copilot", manifest, ext_dir, proj) + + agent_file = proj / ".github" / "agents" / "speckit.ext-cop.cmd.agent.md" + assert agent_file.exists() + parsed = _parse_frontmatter(agent_file.read_text()) + assert "mode" not in parsed, f"mode injected for execution:command: {parsed}" + + +# --------------------------------------------------------------------------- +# Issue 2 — Non-dict behavior in source file bypasses agent-deployment skip +# --------------------------------------------------------------------------- + +class TestExtensionSkillNonDictBehaviorMerge: + """Issue 2: non-dict/empty behavior in source file must not block manifest merge.""" + + @pytest.fixture + def tmp(self): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + def _setup(self, tmp: Path, source_behavior_yaml: str, manifest_behavior: dict | None = None) -> tuple[Path, Path]: + """Create ext + project with ai=codex+ai_skills, command has given behavior in source.""" + cmd_manifest_entry: dict = {"name": "speckit.ext2.cmd", "file": "commands/cmd.md"} + if manifest_behavior is not None: + cmd_manifest_entry["behavior"] = manifest_behavior + + ext_dir = _make_ext_dir(tmp, "ext2", [cmd_manifest_entry]) + + cmd_content = f"---\ndescription: Test\n{source_behavior_yaml}---\n\nBody" + (ext_dir / "commands" / "cmd.md").write_text(cmd_content) + + proj = _make_project(tmp, ai="codex", ai_skills=True) + skills_dir = proj / ".agents" / "skills" + skills_dir.mkdir(parents=True) + return ext_dir, proj + + def test_string_behavior_in_source_allows_manifest_agent_skip(self, tmp): + """Source behavior: 'string' + manifest execution:agent → command skipped (not written as skill).""" + ext_dir, proj = self._setup( + tmp, + source_behavior_yaml="behavior: invalid-string\n", + manifest_behavior={"execution": "agent"}, + ) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + skills = mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".agents" / "skills" / "speckit-ext2-cmd" / "SKILL.md" + assert not skill_file.exists(), ( + "skill was written for a command that should be deployed as an agent definition" + ) + assert skills == [], f"expected empty skills list, got: {skills}" + + def test_null_behavior_in_source_allows_manifest_agent_skip(self, tmp): + """Source behavior: null + manifest execution:agent → command skipped.""" + ext_dir, proj = self._setup( + tmp, + source_behavior_yaml="behavior: null\n", + manifest_behavior={"execution": "agent"}, + ) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + skills = mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".agents" / "skills" / "speckit-ext2-cmd" / "SKILL.md" + assert not skill_file.exists() + + def test_empty_dict_behavior_in_source_allows_manifest_agent_skip(self, tmp): + """Source behavior: {} + manifest execution:agent → command skipped.""" + ext_dir, proj = self._setup( + tmp, + source_behavior_yaml="behavior: {}\n", + manifest_behavior={"execution": "agent"}, + ) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + skills = mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".agents" / "skills" / "speckit-ext2-cmd" / "SKILL.md" + assert not skill_file.exists() + + def test_list_behavior_in_source_allows_manifest_agent_skip(self, tmp): + """Source behavior: [list] + manifest execution:agent → command skipped.""" + ext_dir, proj = self._setup( + tmp, + source_behavior_yaml="behavior:\n - item1\n - item2\n", + manifest_behavior={"execution": "agent"}, + ) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + skills = mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".agents" / "skills" / "speckit-ext2-cmd" / "SKILL.md" + assert not skill_file.exists() + + def test_valid_behavior_in_source_blocks_manifest_merge(self, tmp): + """Source has valid behavior dict → manifest behavior not merged (source wins).""" + ext_dir, proj = self._setup( + tmp, + source_behavior_yaml="behavior:\n execution: isolated\n", + manifest_behavior={"execution": "agent"}, + ) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + skills = mgr._register_extension_skills(manifest, ext_dir) + + # execution:isolated → NOT agent-type → skill IS written + skill_file = proj / ".agents" / "skills" / "speckit-ext2-cmd" / "SKILL.md" + assert skill_file.exists(), "skill should be written when source behavior is not execution:agent" + + def test_no_behavior_no_manifest_behavior_writes_skill(self, tmp): + """No behavior anywhere → normal skill is written.""" + ext_dir, proj = self._setup(tmp, source_behavior_yaml="") + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".agents" / "skills" / "speckit-ext2-cmd" / "SKILL.md" + assert skill_file.exists() + + +class TestExtensionSkillFrontmatterPropagation: + """Issue 6: extension skill rendering must preserve merged source frontmatter.""" + + @pytest.fixture + def tmp(self): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + def test_manifest_behavior_is_applied_to_skill_frontmatter(self, tmp): + """Manifest-level behavior must affect generated SKILL.md frontmatter.""" + ext_dir = _make_ext_dir(tmp, "ext3", [{ + "name": "speckit.ext3.cmd", + "file": "commands/cmd.md", + "behavior": {"invocation": "automatic"}, + }]) + (ext_dir / "commands" / "cmd.md").write_text( + "---\n" + "description: Test\n" + "---\n\n" + "Body" + ) + + proj = _make_project(tmp, ai="claude", ai_skills=True) + (proj / ".claude" / "skills").mkdir(parents=True) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + created = mgr._register_extension_skills(manifest, ext_dir) + assert created == ["speckit-ext3-cmd"] + + skill_file = proj / ".claude" / "skills" / "speckit-ext3-cmd" / "SKILL.md" + parsed = _parse_frontmatter(skill_file.read_text()) + assert parsed.get("disable-model-invocation") is False, parsed + + def test_source_frontmatter_passthrough_is_preserved(self, tmp): + """Passthrough keys like model must be preserved in generated SKILL.md.""" + ext_dir = _make_ext_dir(tmp, "ext4", [{ + "name": "speckit.ext4.cmd", + "file": "commands/cmd.md", + }]) + (ext_dir / "commands" / "cmd.md").write_text( + "---\n" + "description: Test\n" + "model: claude-opus-4-6\n" + "---\n\n" + "Body" + ) + + proj = _make_project(tmp, ai="claude", ai_skills=True) + (proj / ".claude" / "skills").mkdir(parents=True) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".claude" / "skills" / "speckit-ext4-cmd" / "SKILL.md" + parsed = _parse_frontmatter(skill_file.read_text()) + assert parsed.get("model") == "claude-opus-4-6", parsed + + +# --------------------------------------------------------------------------- +# Issue 3 — SkillsIntegration.setup() ignores agents: override +# --------------------------------------------------------------------------- + +class TestSkillsIntegrationAgentsOverride: + """Issue 3: agents: escape-hatch in template frontmatter must reach translate_behavior.""" + + @pytest.fixture + def tmp(self): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + def _find_skill_files(self, tmp_path: Path) -> list[Path]: + return list(tmp_path.rglob("SKILL.md")) + + def test_agents_override_applied_in_skills_integration(self, tmp): + """agents: {: {...}} escape-hatch must appear in generated SKILL.md.""" + from specify_cli.integrations import get_integration + from specify_cli.integrations.manifest import IntegrationManifest + + templates_dir = ( + Path(__file__).resolve().parent.parent / "templates" / "commands" + ) + if not templates_dir.is_dir(): + pytest.skip("templates/commands not available") + + # Patch one template to include behavior + agents override + src_template = next(iter(templates_dir.glob("*.md")), None) + if src_template is None: + pytest.skip("no template files found") + + original = src_template.read_text() + patched = ( + "---\n" + "description: Test patched template\n" + "behavior:\n" + " invocation: automatic\n" + "agents:\n" + " codex:\n" + " effort: max\n" + "---\n" + original.split("---", 2)[-1] if "---" in original else original + ) + + patched_dir = tmp / "patched_templates" + patched_dir.mkdir() + (patched_dir / src_template.name).write_text(patched) + + integration = get_integration("codex") + if integration is None: + pytest.skip("codex integration not registered") + + m = IntegrationManifest("codex", tmp) + + with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))): + created = integration.setup(tmp, m) + + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) > 0, "no SKILL.md files created" + + for sf in skill_files: + content = sf.read_text() + parsed = _parse_frontmatter(content) + # The agents: override for codex sets effort: max + assert parsed.get("effort") == "max", ( + f"agents: override not applied; got frontmatter: {parsed}" + ) + + def test_agents_override_without_behavior_is_applied(self, tmp): + """agents-only template frontmatter must still apply for the target agent.""" + from specify_cli.integrations import get_integration + from specify_cli.integrations.manifest import IntegrationManifest + + templates_dir = ( + Path(__file__).resolve().parent.parent / "templates" / "commands" + ) + if not templates_dir.is_dir(): + pytest.skip("templates/commands not available") + + src_template = next(iter(templates_dir.glob("*.md")), None) + if src_template is None: + pytest.skip("no template files found") + + original = src_template.read_text() + patched = ( + "---\n" + "description: Test patched template\n" + "agents:\n" + " codex:\n" + " effort: max\n" + "---\n" + original.split("---", 2)[-1] if "---" in original else original + ) + + patched_dir = tmp / "patched_templates_agents_only" + patched_dir.mkdir() + (patched_dir / src_template.name).write_text(patched) + + integration = get_integration("codex") + if integration is None: + pytest.skip("codex integration not registered") + + m = IntegrationManifest("codex", tmp) + with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))): + created = integration.setup(tmp, m) + + skill_files = [f for f in created if f.name == "SKILL.md"] + assert skill_files, "no SKILL.md files created" + for sf in skill_files: + parsed = _parse_frontmatter(sf.read_text()) + assert parsed.get("effort") == "max", parsed + + def test_agents_override_for_other_agent_not_applied(self, tmp): + """agents: override for a different agent must not bleed into codex output.""" + from specify_cli.integrations import get_integration + from specify_cli.integrations.manifest import IntegrationManifest + + templates_dir = ( + Path(__file__).resolve().parent.parent / "templates" / "commands" + ) + if not templates_dir.is_dir(): + pytest.skip("templates/commands not available") + + src_template = next(iter(templates_dir.glob("*.md")), None) + if src_template is None: + pytest.skip("no template files found") + + patched = ( + "---\n" + "description: Test patched template\n" + "behavior:\n" + " invocation: automatic\n" + "agents:\n" + " claude:\n" + " effort: max\n" # override for claude, NOT codex + "---\n" + src_template.read_text().split("---", 2)[-1] + if "---" in src_template.read_text() + else src_template.read_text() + ) + + patched_dir = tmp / "patched_templates2" + patched_dir.mkdir() + (patched_dir / src_template.name).write_text(patched) + + integration = get_integration("codex") + if integration is None: + pytest.skip("codex integration not registered") + + m = IntegrationManifest("codex", tmp) + + with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))): + created = integration.setup(tmp, m) + + skill_files = [f for f in created if f.name == "SKILL.md"] + for sf in skill_files: + parsed = _parse_frontmatter(sf.read_text()) + assert parsed.get("effort") != "max", ( + f"claude-only agents: override bled into codex output: {parsed}" + ) + + def test_empty_agents_override_does_not_crash(self, tmp): + """agents: {} in template frontmatter must not raise.""" + from specify_cli.integrations import get_integration + from specify_cli.integrations.manifest import IntegrationManifest + + templates_dir = ( + Path(__file__).resolve().parent.parent / "templates" / "commands" + ) + if not templates_dir.is_dir(): + pytest.skip("templates/commands not available") + + src_template = next(iter(templates_dir.glob("*.md")), None) + if src_template is None: + pytest.skip("no template files found") + + original_text = src_template.read_text() + patched = ( + "---\ndescription: Test template\nbehavior:\n invocation: automatic\nagents: {}\n---\n" + + original_text.split("---", 2)[-1] if "---" in original_text else original_text + ) + + patched_dir = tmp / "patched_templates3" + patched_dir.mkdir() + (patched_dir / src_template.name).write_text(patched) + + integration = get_integration("codex") + if integration is None: + pytest.skip("codex integration not registered") + + m = IntegrationManifest("codex", tmp) + + with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))): + created = integration.setup(tmp, m) # must not raise + + assert len(created) > 0 + + def test_agents_override_non_scalar_yaml_remains_valid(self, tmp): + """Non-scalar override values must be emitted as valid YAML frontmatter.""" + from specify_cli.integrations import get_integration + from specify_cli.integrations.manifest import IntegrationManifest + + templates_dir = ( + Path(__file__).resolve().parent.parent / "templates" / "commands" + ) + if not templates_dir.is_dir(): + pytest.skip("templates/commands not available") + + src_template = next(iter(templates_dir.glob("*.md")), None) + if src_template is None: + pytest.skip("no template files found") + + original_text = src_template.read_text() + patched = ( + "---\n" + "description: Test template\n" + "agents:\n" + " claude:\n" + " handoffs:\n" + " - label: Generate plan\n" + " agent: speckit.plan\n" + " send: true\n" + "---\n" + + original_text.split("---", 2)[-1] + if "---" in original_text + else original_text + ) + + patched_dir = tmp / "patched_templates4" + patched_dir.mkdir() + (patched_dir / src_template.name).write_text(patched) + + integration = get_integration("claude") + if integration is None: + pytest.skip("claude integration not registered") + + m = IntegrationManifest("claude", tmp) + with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))): + created = integration.setup(tmp, m) + + skill_files = [f for f in created if f.name == "SKILL.md"] + assert skill_files, "no SKILL.md files created" + for sf in skill_files: + parsed = _parse_frontmatter(sf.read_text()) + handoffs = parsed.get("handoffs") + assert isinstance(handoffs, list), parsed + assert handoffs[0].get("agent") == "speckit.plan", parsed + + +# --------------------------------------------------------------------------- +# Issue 4 — _behavior_overridable clobbers explicit source frontmatter +# --------------------------------------------------------------------------- + +class TestBehaviorOverridableScope: + """Issue 4: source frontmatter values for model/context/effort/agent/allowed-tools must win.""" + + def _make_registrar_and_source_dir(self, tmp: Path): + from specify_cli.agents import CommandRegistrar as AgentsRegistrar + + src_dir = tmp / "ext-src" + src_dir.mkdir() + (src_dir / "extension.yml").write_text("id: test-ext\n") + return AgentsRegistrar(), src_dir + + @pytest.fixture + def tmp(self): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + def test_source_model_wins_over_behavior_capability(self, tmp): + """Explicit model in source frontmatter must not be clobbered by behavior.capability.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + frontmatter = { + "description": "test", + "model": "claude-opus-4-6", # explicit — must survive + "behavior": {"capability": "fast"}, # would inject claude-haiku-4-5-20251001 + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("model") == "claude-opus-4-6", ( + f"source model was clobbered by behavior.capability; got: {parsed.get('model')}" + ) + + def test_source_effort_wins_over_behavior_effort(self, tmp): + """Explicit effort in source frontmatter must not be clobbered by behavior.effort.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + frontmatter = { + "description": "test", + "effort": "max", # explicit — must survive + "behavior": {"effort": "low"}, # would inject low + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("effort") == "max", ( + f"source effort was clobbered by behavior.effort; got: {parsed.get('effort')}" + ) + + def test_source_context_wins_over_behavior_execution(self, tmp): + """Explicit context in source frontmatter must not be clobbered by behavior.execution.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + frontmatter = { + "description": "test", + "context": "fork", # explicit — must survive + "behavior": {"execution": "command"}, # would produce no injection, but guards + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("context") == "fork", ( + f"source context was clobbered; got: {parsed.get('context')}" + ) + + def test_source_allowed_tools_wins_over_behavior_tools(self, tmp): + """Explicit allowed-tools in source must not be clobbered by behavior.tools.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + frontmatter = { + "description": "test", + "allowed-tools": "Bash", # explicit — must survive + "behavior": {"tools": "read-only"}, # would inject Read Grep Glob + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("allowed-tools") == "Bash", ( + f"source allowed-tools was clobbered; got: {parsed.get('allowed-tools')}" + ) + + def test_behavior_disable_model_invocation_can_override_default(self, tmp): + """behavior.invocation:automatic must be able to set disable-model-invocation:false.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + # Source has no explicit disable-model-invocation; behavior says automatic. + # build_skill_frontmatter injects default True, behavior should override to False. + frontmatter = { + "description": "test", + "behavior": {"invocation": "automatic"}, + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("disable-model-invocation") is False, ( + f"behavior.invocation:automatic did not override default; got: {parsed}" + ) + + def test_behavior_user_invocable_can_override_default(self, tmp): + """behavior.visibility:model must be able to set user-invocable:false.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + frontmatter = { + "description": "test", + "behavior": {"visibility": "model"}, + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("user-invocable") is False, ( + f"behavior.visibility:model did not override default; got: {parsed}" + ) + + def test_source_model_wins_even_when_behavior_capability_also_set(self, tmp): + """Full regression: source model is preserved when both source and behavior specify model.""" + registrar, src_dir = self._make_registrar_and_source_dir(tmp) + + frontmatter = { + "description": "test", + "model": "claude-opus-4-6", + "behavior": { + "capability": "fast", # would inject haiku + "invocation": "automatic", # must still work + }, + } + output = registrar.render_skill_command( + "claude", "speckit-test", frontmatter, "Body", "test-ext", + "commands/test.md", tmp, source_dir=src_dir, + ) + parsed = _parse_frontmatter(output) + assert parsed.get("model") == "claude-opus-4-6" + assert parsed.get("disable-model-invocation") is False + + +# --------------------------------------------------------------------------- +# Issue 5 — extensions.py reads source file twice; import inside loop +# --------------------------------------------------------------------------- + +class TestExtensionSkillSingleRead: + """Issue 5: _register_extension_skills must read each source file exactly once.""" + + @pytest.fixture + def tmp(self): + d = tempfile.mkdtemp() + yield Path(d) + shutil.rmtree(d) + + def test_source_file_read_at_most_once(self, tmp): + """Each source command file must be read at most once per registration pass.""" + ext_dir = _make_ext_dir(tmp, "ext5", [ + {"name": "speckit.ext5.cmd", "file": "commands/cmd.md"} + ]) + cmd_file = ext_dir / "commands" / "cmd.md" + cmd_file.write_text("---\ndescription: Test\n---\n\nBody") + + proj = _make_project(tmp, ai="codex", ai_skills=True) + (proj / ".agents" / "skills").mkdir(parents=True) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + + read_calls: list[Path] = [] + original_read_text = Path.read_text + + def counting_read_text(self_path, *args, **kwargs): + if self_path == cmd_file: + read_calls.append(self_path) + return original_read_text(self_path, *args, **kwargs) + + with patch.object(Path, "read_text", counting_read_text): + mgr._register_extension_skills(manifest, ext_dir) + + assert len(read_calls) <= 1, ( + f"source file read {len(read_calls)} times, expected at most 1" + ) + + def test_get_deployment_type_imported_at_module_scope(self): + """get_deployment_type must be importable directly from specify_cli.behavior.""" + from specify_cli.behavior import get_deployment_type + assert callable(get_deployment_type) + + def test_double_read_fix_preserves_correct_behavior(self, tmp): + """After fix, agent-deployment skip still works (no regression from read refactor).""" + ext_dir = _make_ext_dir(tmp, "ext5b", [ + { + "name": "speckit.ext5b.cmd", + "file": "commands/cmd.md", + "behavior": {"execution": "agent"}, + } + ]) + cmd_file = ext_dir / "commands" / "cmd.md" + cmd_file.write_text("---\ndescription: Agent command\n---\n\nBody") + + proj = _make_project(tmp, ai="codex", ai_skills=True) + (proj / ".agents" / "skills").mkdir(parents=True) + + from specify_cli.extensions import ExtensionManifest, ExtensionManager + manifest = ExtensionManifest(ext_dir / "extension.yml") + mgr = ExtensionManager(proj) + skills = mgr._register_extension_skills(manifest, ext_dir) + + skill_file = proj / ".agents" / "skills" / "speckit-ext5b-cmd" / "SKILL.md" + assert not skill_file.exists(), "agent-type command should not produce a SKILL.md" + assert skills == [] From a06dc0f8baa7edc8141250a4198674304afb1cab Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Fri, 1 May 2026 19:27:53 +0200 Subject: [PATCH 2/5] Address PR 2103 review feedback --- src/specify_cli/__init__.py | 5 ++--- src/specify_cli/extensions.py | 4 ++-- tests/test_pr2103_reviews.py | 3 ++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index dae3f76e6d..7485c3c635 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -807,9 +807,8 @@ def _install_shared_infra( for f in skipped_files: console.print(f" {f}") console.print( - "To refresh shared infrastructure, run " - "[cyan]specify init --here --force[/cyan] or " - "[cyan]specify integration upgrade --force[/cyan]." + "To refresh shared infrastructure, run specify init --here --force " + "or specify integration upgrade --force." ) manifest.save() diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 36ee390786..d98d7388f6 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -921,7 +921,7 @@ def _register_extension_skills( # Skip commands that behavior-routing deploys as agent definitions # (to .claude/agents/) rather than as skill files. - _source_fm, _ = registrar.parse_frontmatter(content) + _source_fm, body = registrar.parse_frontmatter(content) # Merge manifest-level behavior when source file has none or has an # invalid (non-dict / empty) value. A non-dict value (string, list, # null) cannot carry execution:agent, so the manifest entry must win @@ -933,7 +933,7 @@ def _register_extension_skills( continue skill_subdir.mkdir(parents=True, exist_ok=True) - frontmatter, body = registrar.parse_frontmatter(content) + frontmatter = _source_fm for key in ("agents",): if key in cmd_info and key not in frontmatter: diff --git a/tests/test_pr2103_reviews.py b/tests/test_pr2103_reviews.py index 46de1fae50..78dce31ffd 100644 --- a/tests/test_pr2103_reviews.py +++ b/tests/test_pr2103_reviews.py @@ -1,11 +1,12 @@ """Tests for issues raised in PR #2103 code review. -Covers five bugs found by Copilot inline review: +Covers six bugs found by Copilot inline review: Issue 1 — Copilot execution:isolated never gets mode:agent injected; behavior/agents keys leak Issue 2 — Non-dict behavior in source file bypasses manifest-level agent-deployment skip Issue 3 — SkillsIntegration.setup() ignores agents: escape-hatch overrides Issue 4 — _behavior_overridable clobbers explicit source frontmatter (model, context, etc.) Issue 5 — extensions.py _register_extension_skills reads source file twice per iteration + Issue 6 — Extension skill rendering drops merged behavior/source frontmatter in output """ import json From b597e645b28b8302c9eb00378fe8359c8420769b Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Fri, 8 May 2026 15:33:45 +0200 Subject: [PATCH 3/5] fix: extension skill path rewrite and security hardening - Add path traversal validation in ExtensionManager._resolve_manifest_relative_path to prevent config files from escaping the extension directory - Add ExtensionManifest.config_files property for accessing provides.config entries - Add auto-commit config schema to git extension with per-phase hooks - Improve git extension scripts (auto-commit.sh, initialize-repo.sh, PS1 variants) with better error handling and branch detection - Update Claude and Copilot integrations for skills-based path rewriting - Add CLI commands for extension config management - Add comprehensive tests for extension path validation and git extension - Update extension documentation (API reference, dev guide, RFC, README) --- extensions/EXTENSION-API-REFERENCE.md | 40 +- extensions/EXTENSION-DEVELOPMENT-GUIDE.md | 35 +- extensions/README.md | 16 + extensions/RFC-EXTENSION-SYSTEM.md | 74 +-- extensions/git/README.md | 35 +- extensions/git/commands/speckit.git.commit.md | 14 +- .../git/commands/speckit.git.feature.md | 16 +- extensions/git/extension.yml | 50 ++ extensions/git/scripts/bash/auto-commit.sh | 50 +- .../git/scripts/bash/initialize-repo.sh | 21 +- .../git/scripts/powershell/auto-commit.ps1 | 46 +- .../scripts/powershell/initialize-repo.ps1 | 25 +- src/specify_cli/__init__.py | 77 +++ src/specify_cli/extensions.py | 95 ++- .../integrations/claude/__init__.py | 34 +- .../integrations/copilot/__init__.py | 72 ++- tests/extensions/git/test_git_extension.py | 353 ++++++++++++ tests/integrations/test_cli.py | 4 +- .../test_integration_subcommand.py | 15 +- tests/test_extensions.py | 543 ++++++++++++++++++ 20 files changed, 1495 insertions(+), 120 deletions(-) diff --git a/extensions/EXTENSION-API-REFERENCE.md b/extensions/EXTENSION-API-REFERENCE.md index ce0ff1775c..813dbc10fb 100644 --- a/extensions/EXTENSION-API-REFERENCE.md +++ b/extensions/EXTENSION-API-REFERENCE.md @@ -48,7 +48,7 @@ provides: config: # Optional, array of config files - name: string # Config file name - template: string # Template file path + template: string # Template file path (copied to `name` on install if missing) description: string required: boolean # Default: false @@ -514,11 +514,30 @@ field_mappings: ### Config Layers -1. **Extension Defaults** (from `extension.yml` `defaults` section) +1. **Extension Defaults** (from `extension.yml` `config.defaults` section) 2. **Project Config** (`{extension-id}-config.yml`) -3. **Local Override** (`{extension-id}-config.local.yml`, gitignored) +3. **Local Override** (`local-config.yml`, gitignored) 4. **Environment Variables** (`SPECKIT_{EXTENSION}_*`) +Backward compatibility: `local.yml` is also loaded as a legacy local-override file, but `local-config.yml` is canonical and takes precedence when both exist. + +### Config Resolution Command + +Use the built-in resolver instead of re-implementing YAML merge logic in each extension: + +```bash +# Emit merged config as JSON +specify extension config resolve jira --format json + +# Emit flattened env assignments for shell scripts +specify extension config resolve jira --format env --prefix JIRA_CFG_ +``` + +For `--format env`, nested keys are flattened with `_`: + +- `project.key` -> `JIRA_CFG_PROJECT_KEY` +- `feature.enabled` -> `JIRA_CFG_FEATURE_ENABLED` + ### Environment Variable Pattern Format: `SPECKIT_{EXTENSION}_{KEY}` @@ -622,6 +641,21 @@ EXECUTE_COMMAND: {command} **Output**: List of installed extensions with metadata +### extension config resolve + +**Usage**: `specify extension config resolve EXTENSION [OPTIONS]` + +Resolves layered extension configuration and emits either JSON or shell env assignments. + +**Options**: + +- `--format [json|env]` - Output mode (default: `json`) +- `--prefix TEXT` - Env key prefix for `--format env` (default: `EXTCFG_`) + +**Arguments**: + +- `EXTENSION` - Installed extension ID or display name + ### extension catalog list **Usage**: `specify extension catalog list` diff --git a/extensions/EXTENSION-DEVELOPMENT-GUIDE.md b/extensions/EXTENSION-DEVELOPMENT-GUIDE.md index 42ce2d71df..ab5121be3f 100644 --- a/extensions/EXTENSION-DEVELOPMENT-GUIDE.md +++ b/extensions/EXTENSION-DEVELOPMENT-GUIDE.md @@ -311,30 +311,39 @@ credentials: api_key: "${MY_EXT_API_KEY}" ``` +When the extension is installed, each `provides.config` entry copies its +`template` file to `name` if the target file does not already exist. + ### Config Loading -In your command, load config with layered precedence: +Use the built-in resolver command to load layered config: -1. Extension defaults (`extension.yml` → `defaults`) +1. Extension defaults (`extension.yml` → `config.defaults`) 2. Project config (`.specify/extensions/my-ext/my-ext-config.yml`) -3. Local overrides (`.specify/extensions/my-ext/my-ext-config.local.yml` - gitignored) +3. Local overrides (`.specify/extensions/my-ext/local-config.yml` - gitignored) 4. Environment variables (`SPECKIT_MY_EXT_*`) -**Example loading script**: +Note: `local.yml` is still read for backward compatibility, but `local-config.yml` is the canonical filename and wins when both are present. + +**Recommended command usage**: ```bash -#!/usr/bin/env bash -EXT_DIR=".specify/extensions/my-ext" +# JSON for structured consumers +config_json=$(specify extension config resolve my-ext --format json) -# Load and merge config -config=$(yq eval '.' "$EXT_DIR/my-ext-config.yml" -o=json) +# Or flattened env vars for shell scripts +eval "$(specify extension config resolve my-ext --format env --prefix MY_EXT_CFG_)" +``` -# Apply env overrides -if [ -n "${SPECKIT_MY_EXT_API_KEY:-}" ]; then - config=$(echo "$config" | jq ".api.api_key = \"$SPECKIT_MY_EXT_API_KEY\"") -fi +**Example script usage after env export**: + +```bash +#!/usr/bin/env bash +set -euo pipefail -echo "$config" +eval "$(specify extension config resolve my-ext --format env --prefix MY_EXT_CFG_)" +echo "Endpoint: ${MY_EXT_CFG_API_ENDPOINT:-}" +echo "Timeout: ${MY_EXT_CFG_API_TIMEOUT:-30}" ``` --- diff --git a/extensions/README.md b/extensions/README.md index f535ba539a..430c54fb70 100644 --- a/extensions/README.md +++ b/extensions/README.md @@ -122,4 +122,20 @@ specify extension add --from https://github.com///ar specify extension list ``` +## Resolving Extension Config + +Extensions can expose layered config (defaults + project file + local overrides + env vars). +Use the built-in resolver instead of hand-rolling merge logic in every script: + +```bash +# Emit merged config as JSON +specify extension config resolve --format json + +# Emit flattened env assignments for shell scripts +eval "$(specify extension config resolve --format env --prefix EXT_CFG_)" +``` + +When an extension declares `provides.config` entries, install now materializes each +declared `name` from its `template` when the target file is missing. + For more information, see the [Extension User Guide](EXTENSION-USER-GUIDE.md). diff --git a/extensions/RFC-EXTENSION-SYSTEM.md b/extensions/RFC-EXTENSION-SYSTEM.md index 6c4b87cc30..843df342c7 100644 --- a/extensions/RFC-EXTENSION-SYSTEM.md +++ b/extensions/RFC-EXTENSION-SYSTEM.md @@ -249,14 +249,15 @@ provides: executable: true # Make executable on install # Extension configuration defaults (OPTIONAL) -defaults: - project: - key: null # No default, user must configure - hierarchy: - issue_type: "subtask" - update_behavior: - mode: "update" - sync_completion: true +config: + defaults: + project: + key: null # No default, user must configure + hierarchy: + issue_type: "subtask" + update_behavior: + mode: "update" + sync_completion: true # Configuration schema for validation (OPTIONAL) config_schema: @@ -378,9 +379,9 @@ vim .specify/extensions/jira/jira-config.yml **Config discovery order:** -1. Extension defaults (`extension.yml` → `defaults`) +1. Extension defaults (`extension.yml` → `config.defaults`) 2. Project config (`jira-config.yml`) -3. Local overrides (`jira-config.local.yml` - gitignored) +3. Local overrides (`local-config.yml` - gitignored; legacy `local.yml` also supported) 4. Environment variables (`SPECKIT_JIRA_*`) ### 4. Usage @@ -613,14 +614,10 @@ project: hierarchy: issue_type: "subtask" - -defaults: - epic: - labels: ["spec-driven", "typescript"] ``` ```yaml -# .specify/extensions/jira/jira-config.local.yml (Local overrides - gitignored) +# .specify/extensions/jira/local-config.yml (Local overrides - gitignored) project: key: "MYTEST" # Override for local testing ``` @@ -637,50 +634,25 @@ export SPECKIT_JIRA_PROJECT_KEY="DEVTEST" ````markdown ## Load Configuration -1. Run helper script to load and merge config: +1. Use built-in config resolver (single source of truth): ```bash -config_json=$(bash .specify/extensions/jira/scripts/parse-jira-config.sh) +config_json=$(specify extension config resolve jira --format json) echo "$config_json" ``` 1. Parse JSON and use in subsequent steps ```` -**Script**: `.specify/extensions/jira/scripts/parse-jira-config.sh` +For shell-first scripts, emit flattened env assignments: ```bash -#!/usr/bin/env bash -set -euo pipefail - -EXT_DIR=".specify/extensions/jira" -CONFIG_FILE="$EXT_DIR/jira-config.yml" -LOCAL_CONFIG="$EXT_DIR/jira-config.local.yml" - -# Start with defaults from extension.yml -defaults=$(yq eval '.defaults' "$EXT_DIR/extension.yml" -o=json) - -# Merge project config -if [ -f "$CONFIG_FILE" ]; then - project_config=$(yq eval '.' "$CONFIG_FILE" -o=json) - defaults=$(echo "$defaults $project_config" | jq -s '.[0] * .[1]') -fi - -# Merge local config -if [ -f "$LOCAL_CONFIG" ]; then - local_config=$(yq eval '.' "$LOCAL_CONFIG" -o=json) - defaults=$(echo "$defaults $local_config" | jq -s '.[0] * .[1]') -fi - -# Apply environment variable overrides -if [ -n "${SPECKIT_JIRA_PROJECT_KEY:-}" ]; then - defaults=$(echo "$defaults" | jq ".project.key = \"$SPECKIT_JIRA_PROJECT_KEY\"") -fi - -# Output merged config as JSON -echo "$defaults" +eval "$(specify extension config resolve jira --format env --prefix JIRA_CFG_)" +echo "project_key=${JIRA_CFG_PROJECT_KEY:-}" ``` +This avoids re-implementing YAML merge/override logic in every extension script. + ### Config Validation **In command file**: @@ -694,12 +666,14 @@ echo "$defaults" ```python import jsonschema -schema = load_yaml(".specify/extensions/jira/extension.yml")['config_schema'] +manifest = load_yaml(".specify/extensions/jira/extension.yml") +schema = manifest.get("config_schema") config = json.loads(config_json) -try: +if schema: + try: jsonschema.validate(config, schema) -except jsonschema.ValidationError as e: + except jsonschema.ValidationError as e: print(f"❌ Invalid jira-config.yml: {e.message}") print(f" Path: {'.'.join(str(p) for p in e.path)}") exit(1) diff --git a/extensions/git/README.md b/extensions/git/README.md index 31ba75c30f..849b842e8f 100644 --- a/extensions/git/README.md +++ b/extensions/git/README.md @@ -47,7 +47,26 @@ This extension provides Git operations as an optional, self-contained module. It ## Configuration -Configuration is stored in `.specify/extensions/git/git-config.yml`: +Configuration uses a layered model (lowest to highest precedence): + +1. **Manifest defaults** — baked into `extension.yml` under `config.defaults` +2. **Project config** — `.specify/extensions/git/git-config.yml` (committed, shared with team) +3. **Local override** — `.specify/extensions/git/local-config.yml` (gitignored, machine-specific) +4. **Environment variables** — `SPECKIT_GIT_` overrides all files + +### Resolving config at runtime + +```bash +# JSON output (structured consumers) +specify extension config resolve git --format json + +# Flat env export for shell scripts +# shellcheck disable=SC2046 +eval "$(specify extension config resolve git --format env --prefix GIT_CFG_)" +echo "Numbering: ${GIT_CFG_BRANCH_NUMBERING:-sequential}" +``` + +### Example project config (`.specify/extensions/git/git-config.yml`) ```yaml # Branch numbering strategy: "sequential" or "timestamp" @@ -65,6 +84,20 @@ auto_commit: message: "[Spec Kit] Add specification" ``` +### Example local override (`.specify/extensions/git/local-config.yml`, gitignored) + +```yaml +# Override branch numbering for this machine only +branch_numbering: timestamp +``` + +### Environment variable override + +```bash +# Override branch numbering via env (highest precedence) +export SPECKIT_GIT_BRANCH_NUMBERING=timestamp +``` + ## Installation ```bash diff --git a/extensions/git/commands/speckit.git.commit.md b/extensions/git/commands/speckit.git.commit.md index e606f911df..9b94e909a4 100644 --- a/extensions/git/commands/speckit.git.commit.md +++ b/extensions/git/commands/speckit.git.commit.md @@ -11,7 +11,7 @@ Automatically stage and commit all changes after a Spec Kit command completes. This command is invoked as a hook after (or before) core commands. It: 1. Determines the event name from the hook context (e.g., if invoked as an `after_specify` hook, the event is `after_specify`; if `before_plan`, the event is `before_plan`) -2. Checks `.specify/extensions/git/git-config.yml` for the `auto_commit` section +2. Resolves extension configuration via the CLI resolver to obtain the `auto_commit` section 3. Looks up the specific event key to see if auto-commit is enabled 4. Falls back to `auto_commit.default` if no event-specific key exists 5. Uses the per-command `message` if configured, otherwise a default message @@ -28,9 +28,19 @@ Replace `` with the actual hook event (e.g., `after_specify`, `befor ## Configuration -In `.specify/extensions/git/git-config.yml`: +Resolve the config before invoking the script so scripts receive values as environment variables: + +```bash +# shellcheck disable=SC2046 +# eval is required here because resolver emits KEY=VALUE lines +# for the current shell session. +eval "$(specify extension config resolve git --format env --prefix GIT_CFG_)" +``` + +Defaults and overrides are managed in `git-config.yml` (project-level) and `local-config.yml` (machine-local, gitignored). Example project config: ```yaml +# .specify/extensions/git/git-config.yml auto_commit: default: false # Global toggle — set true to enable for all commands after_specify: diff --git a/extensions/git/commands/speckit.git.feature.md b/extensions/git/commands/speckit.git.feature.md index 1a9c5e35da..775135d075 100644 --- a/extensions/git/commands/speckit.git.feature.md +++ b/extensions/git/commands/speckit.git.feature.md @@ -28,11 +28,21 @@ If the user explicitly provided `GIT_BRANCH_NAME` (e.g., via environment variabl ## Branch Numbering Mode -Determine the branch numbering strategy by checking configuration in this order: +Determine the branch numbering strategy by resolving the extension configuration: + +```bash +# shellcheck disable=SC2046 +# eval is required here because resolver emits KEY=VALUE lines +# for the current shell session. +eval "$(specify extension config resolve git --format env --prefix GIT_CFG_)" +BRANCH_NUMBERING="${GIT_CFG_BRANCH_NUMBERING:-sequential}" +``` + +For backward compatibility, if `specify` is unavailable, fall back to: 1. Check `.specify/extensions/git/git-config.yml` for `branch_numbering` value -2. Check `.specify/init-options.json` for `branch_numbering` value (backward compatibility) -3. Default to `sequential` if neither exists +2. Check `.specify/init-options.json` for `branch_numbering` value +3. Default to `sequential` ## Execution diff --git a/extensions/git/extension.yml b/extensions/git/extension.yml index 13c1977ea1..fccb47e969 100644 --- a/extensions/git/extension.yml +++ b/extensions/git/extension.yml @@ -138,3 +138,53 @@ config: defaults: branch_numbering: sequential init_commit_message: "[Spec Kit] Initial commit" + auto_commit: + default: false + before_clarify: + enabled: false + message: "[Spec Kit] Save progress before clarification" + before_plan: + enabled: false + message: "[Spec Kit] Save progress before planning" + before_tasks: + enabled: false + message: "[Spec Kit] Save progress before task generation" + before_implement: + enabled: false + message: "[Spec Kit] Save progress before implementation" + before_checklist: + enabled: false + message: "[Spec Kit] Save progress before checklist" + before_analyze: + enabled: false + message: "[Spec Kit] Save progress before analysis" + before_taskstoissues: + enabled: false + message: "[Spec Kit] Save progress before issue sync" + after_constitution: + enabled: false + message: "[Spec Kit] Add project constitution" + after_specify: + enabled: false + message: "[Spec Kit] Add specification" + after_clarify: + enabled: false + message: "[Spec Kit] Clarify specification" + after_plan: + enabled: false + message: "[Spec Kit] Add implementation plan" + after_tasks: + enabled: false + message: "[Spec Kit] Add tasks" + after_implement: + enabled: false + message: "[Spec Kit] Implementation progress" + after_checklist: + enabled: false + message: "[Spec Kit] Add checklist" + after_analyze: + enabled: false + message: "[Spec Kit] Add analysis report" + after_taskstoissues: + enabled: false + message: "[Spec Kit] Sync tasks to issues" diff --git a/extensions/git/scripts/bash/auto-commit.sh b/extensions/git/scripts/bash/auto-commit.sh index f0b423187b..ef35404c25 100755 --- a/extensions/git/scripts/bash/auto-commit.sh +++ b/extensions/git/scripts/bash/auto-commit.sh @@ -42,12 +42,53 @@ if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then exit 0 fi -# Read per-command config from git-config.yml -_config_file="$REPO_ROOT/.specify/extensions/git/git-config.yml" +# Resolve per-command config. +# +# Preferred path: caller exports GIT_CFG_* via the config resolver before +# invoking this script: +# eval "$(specify extension config resolve git --format env --prefix GIT_CFG_)" +# +# The resolver flattens nested YAML keys with underscores, so: +# auto_commit..enabled -> GIT_CFG_AUTO_COMMIT__ENABLED +# auto_commit..message -> GIT_CFG_AUTO_COMMIT__MESSAGE +# auto_commit.default -> GIT_CFG_AUTO_COMMIT_DEFAULT +# +# Fallback path: if GIT_CFG_* vars are absent, parse git-config.yml directly +# (backward compatibility). + _enabled=false _commit_msg="" -if [ -f "$_config_file" ]; then +# Build the env-var key fragment for this event (upper-case, hyphens->underscores) +_EVENT_KEY=$(echo "$EVENT_NAME" | tr '[:lower:]' '[:upper:]' | tr '-' '_') + +# Check whether resolver-provided env vars are present for this event +_env_prefix_event="GIT_CFG_AUTO_COMMIT_${_EVENT_KEY}_" +_env_enabled_var="${_env_prefix_event}ENABLED" +_env_msg_var="${_env_prefix_event}MESSAGE" +_env_default_var="GIT_CFG_AUTO_COMMIT_DEFAULT" + +if [ -n "${!_env_enabled_var+x}" ] || [ -n "${!_env_default_var+x}" ]; then + # Resolver env vars are present — consume them + _event_enabled="${!_env_enabled_var:-}" + _default_enabled="${!_env_default_var:-false}" + + if [ -n "$_event_enabled" ]; then + [ "$_event_enabled" = "true" ] && _enabled=true || _enabled=false + elif [ "$_default_enabled" = "true" ]; then + _enabled=true + fi + + _commit_msg="${!_env_msg_var:-}" +else + # Fallback: parse git-config.yml directly + _config_file="$REPO_ROOT/.specify/extensions/git/git-config.yml" + + if [ ! -f "$_config_file" ]; then + # No config file — auto-commit disabled by default + exit 0 + fi + # Parse the auto_commit section for this event. # Look for auto_commit..enabled and .message # Also check auto_commit.default as fallback. @@ -108,9 +149,6 @@ if [ -f "$_config_file" ]; then _enabled=true fi fi -else - # No config file — auto-commit disabled by default - exit 0 fi if [ "$_enabled" != "true" ]; then diff --git a/extensions/git/scripts/bash/initialize-repo.sh b/extensions/git/scripts/bash/initialize-repo.sh index 296e363b94..9228520a8c 100755 --- a/extensions/git/scripts/bash/initialize-repo.sh +++ b/extensions/git/scripts/bash/initialize-repo.sh @@ -24,15 +24,22 @@ _find_project_root() { REPO_ROOT=$(_find_project_root "$SCRIPT_DIR") || REPO_ROOT="$(pwd)" cd "$REPO_ROOT" -# Read commit message from extension config, fall back to default -COMMIT_MSG="[Spec Kit] Initial commit" -_config_file="$REPO_ROOT/.specify/extensions/git/git-config.yml" -if [ -f "$_config_file" ]; then - _msg=$(grep '^init_commit_message:' "$_config_file" 2>/dev/null | sed 's/^init_commit_message:[[:space:]]*//' | sed 's/^["'\'']//' | sed 's/["'\'']*$//') - if [ -n "$_msg" ]; then - COMMIT_MSG="$_msg" +# Resolve commit message: env var set by caller (via config resolver) takes priority, +# then fall back to ad-hoc YAML read for backward compatibility, then hardcoded default. +# +# Callers should export GIT_CFG_INIT_COMMIT_MESSAGE before invoking this script: +# eval "$(specify extension config resolve git --format env --prefix GIT_CFG_)" +COMMIT_MSG="${GIT_CFG_INIT_COMMIT_MESSAGE:-}" +if [ -z "$COMMIT_MSG" ]; then + _config_file="$REPO_ROOT/.specify/extensions/git/git-config.yml" + if [ -f "$_config_file" ]; then + _msg=$(grep '^init_commit_message:' "$_config_file" 2>/dev/null | sed 's/^init_commit_message:[[:space:]]*//' | sed 's/^["'\'']//' | sed 's/["'\'']*$//') + if [ -n "$_msg" ]; then + COMMIT_MSG="$_msg" + fi fi fi +: "${COMMIT_MSG:=[Spec Kit] Initial commit}" # Check if git is available if ! command -v git >/dev/null 2>&1; then diff --git a/extensions/git/scripts/powershell/auto-commit.ps1 b/extensions/git/scripts/powershell/auto-commit.ps1 index 4a8b0e00cd..fca6acafdf 100644 --- a/extensions/git/scripts/powershell/auto-commit.ps1 +++ b/extensions/git/scripts/powershell/auto-commit.ps1 @@ -51,12 +51,49 @@ if (-not $isRepo) { exit 0 } -# Read per-command config from git-config.yml -$configFile = Join-Path $repoRoot ".specify/extensions/git/git-config.yml" +# Resolve per-command config. +# +# Preferred path: caller exports GIT_CFG_* via the config resolver before +# invoking this script: +# specify extension config resolve git --format env --prefix GIT_CFG_ | Invoke-Expression +# +# The resolver flattens nested YAML keys with underscores, so: +# auto_commit..enabled -> GIT_CFG_AUTO_COMMIT__ENABLED +# auto_commit..message -> GIT_CFG_AUTO_COMMIT__MESSAGE +# auto_commit.default -> GIT_CFG_AUTO_COMMIT_DEFAULT +# +# Fallback path: if GIT_CFG_* vars are absent, parse git-config.yml directly +# (backward compatibility). + $enabled = $false $commitMsg = "" -if (Test-Path $configFile) { +# Build the env-var key fragment for this event (upper-case, hyphens->underscores) +$eventKey = $EventName.ToUpper() -replace '-', '_' +$envEnabledVar = "GIT_CFG_AUTO_COMMIT_${eventKey}_ENABLED" +$envMsgVar = "GIT_CFG_AUTO_COMMIT_${eventKey}_MESSAGE" +$envDefaultVar = "GIT_CFG_AUTO_COMMIT_DEFAULT" + +$envEnabledVal = [System.Environment]::GetEnvironmentVariable($envEnabledVar) +$envDefaultVal = [System.Environment]::GetEnvironmentVariable($envDefaultVar) + +if ($null -ne $envEnabledVal -or $null -ne $envDefaultVal) { + # Resolver env vars are present — consume them + if ($null -ne $envEnabledVal) { + $enabled = ($envEnabledVal.Trim().ToLower() -eq 'true') + } elseif ($null -ne $envDefaultVal -and $envDefaultVal.Trim().ToLower() -eq 'true') { + $enabled = $true + } + $commitMsg = [System.Environment]::GetEnvironmentVariable($envMsgVar) ?? "" +} else { + # Fallback: parse git-config.yml directly + $configFile = Join-Path $repoRoot ".specify/extensions/git/git-config.yml" + + if (-not (Test-Path $configFile)) { + # No config file — auto-commit disabled by default + exit 0 + } + # Parse YAML to find auto_commit section $inAutoCommit = $false $inEvent = $false @@ -114,9 +151,6 @@ if (Test-Path $configFile) { $enabled = $true } } -} else { - # No config file — auto-commit disabled by default - exit 0 } if (-not $enabled) { diff --git a/extensions/git/scripts/powershell/initialize-repo.ps1 b/extensions/git/scripts/powershell/initialize-repo.ps1 index 324240a3e7..d3aa738b42 100644 --- a/extensions/git/scripts/powershell/initialize-repo.ps1 +++ b/extensions/git/scripts/powershell/initialize-repo.ps1 @@ -25,18 +25,25 @@ $repoRoot = Find-ProjectRoot -StartDir $PSScriptRoot if (-not $repoRoot) { $repoRoot = Get-Location } Set-Location $repoRoot -# Read commit message from extension config, fall back to default -$commitMsg = "[Spec Kit] Initial commit" -$configFile = Join-Path $repoRoot ".specify/extensions/git/git-config.yml" -if (Test-Path $configFile) { - foreach ($line in Get-Content $configFile) { - if ($line -match '^init_commit_message:\s*(.+)$') { - $val = $matches[1].Trim() -replace '^["'']' -replace '["'']$' - if ($val) { $commitMsg = $val } - break +# Resolve commit message: env var set by caller (via config resolver) takes priority, +# then fall back to ad-hoc YAML read for backward compatibility, then hardcoded default. +# +# Callers should export GIT_CFG_INIT_COMMIT_MESSAGE before invoking this script: +# specify extension config resolve git --format env --prefix GIT_CFG_ | Invoke-Expression +$commitMsg = $env:GIT_CFG_INIT_COMMIT_MESSAGE +if (-not $commitMsg) { + $configFile = Join-Path $repoRoot ".specify/extensions/git/git-config.yml" + if (Test-Path $configFile) { + foreach ($line in Get-Content $configFile) { + if ($line -match '^init_commit_message:\s*(.+)$') { + $val = $matches[1].Trim() -replace '^["'']' -replace '["'']$' + if ($val) { $commitMsg = $val } + break + } } } } +if (-not $commitMsg) { $commitMsg = "[Spec Kit] Initial commit" } # Check if git is available if (-not (Get-Command git -ErrorAction SilentlyContinue)) { diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index bee7977cfc..58404a3658 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1871,6 +1871,13 @@ def self_upgrade() -> None: ) extension_app.add_typer(catalog_app, name="catalog") +extension_config_app = typer.Typer( + name="config", + help="Resolve extension configuration", + add_completion=False, +) +extension_app.add_typer(extension_config_app, name="config") + preset_app = typer.Typer( name="preset", help="Manage spec-kit presets", @@ -3899,6 +3906,76 @@ def preset_catalog_remove( # ===== Extension Commands ===== +def _flatten_config_items(config: Any, key_parts: tuple[str, ...] = ()): + """Yield flattened key-path tuples for scalar/list/dict config leaf values.""" + if isinstance(config, dict): + for key in sorted(config.keys()): + value = config[key] + if not isinstance(key, str): + key = str(key) + yield from _flatten_config_items(value, (*key_parts, key)) + else: + if key_parts: + yield key_parts, config + + +def _config_value_to_env_string(value: Any) -> str: + """Convert resolved config values to deterministic shell-friendly strings.""" + if isinstance(value, bool): + return "true" if value else "false" + if value is None: + return "" + if isinstance(value, (int, float, str)): + return str(value) + return json.dumps(value, sort_keys=True, separators=(",", ":")) + + +@extension_config_app.command("resolve") +def extension_config_resolve( + extension: str = typer.Argument(help="Installed extension ID or display name"), + format: str = typer.Option("json", "--format", help="Output format: json | env"), + prefix: str = typer.Option("EXTCFG_", "--prefix", help="Prefix for --format env keys"), +): + """Resolve layered extension config and emit it for commands/scripts. + + Layers (low -> high): manifest defaults, project config, local config, env. + """ + from .extensions import ExtensionManager, ConfigManager + + output_format = format.lower().strip() + if output_format not in {"json", "env"}: + console.print("[red]Error:[/red] --format must be one of: json, env") + raise typer.Exit(1) + + project_root = _require_specify_project() + manager = ExtensionManager(project_root) + installed = manager.list_installed() + extension_id, _ = _resolve_installed_extension( + extension, installed, "config resolve" + ) + + config = ConfigManager(project_root, extension_id).get_config() + + if output_format == "json": + typer.echo(json.dumps(config, indent=2, sort_keys=True)) + return + + env_prefix = prefix.strip().upper() + if not env_prefix: + console.print("[red]Error:[/red] --prefix must not be empty when using --format env") + raise typer.Exit(1) + if not env_prefix.endswith("_"): + env_prefix += "_" + + lines: list[str] = [] + for key_parts, value in _flatten_config_items(config): + env_key = env_prefix + "_".join(part.upper() for part in key_parts) + env_value = _config_value_to_env_string(value) + lines.append(f"{env_key}={shlex.quote(env_value)}") + + typer.echo("\n".join(lines)) + + def _resolve_installed_extension( argument: str, installed_extensions: list, diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index c877c62830..9b412895db 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -362,6 +362,19 @@ def hooks(self) -> Dict[str, Any]: """Get hook definitions.""" return self.data.get("hooks", {}) + @property + def config_files(self) -> List[Dict[str, Any]]: + """Get configured extension config-file entries. + + Returns: + List of entries from provides.config, or an empty list. + """ + provides = self.data.get("provides", {}) + if not isinstance(provides, dict): + return [] + config_entries = provides.get("config", []) + return config_entries if isinstance(config_entries, list) else [] + def get_hash(self) -> str: """Calculate SHA256 hash of manifest file.""" with open(self.path, 'rb') as f: @@ -806,6 +819,75 @@ def _ignore(directory: str, entries: List[str]) -> Set[str]: return _ignore + @staticmethod + def _resolve_manifest_relative_path( + extension_dir: Path, + relative_path: str, + field_name: str, + ) -> Path: + """Resolve and validate a manifest relative path stays within extension root.""" + rel = Path(relative_path) + if rel.is_absolute(): + raise ValidationError(f"{field_name} must be a relative path: {relative_path!r}") + + try: + ext_root = extension_dir.resolve() + resolved = (ext_root / rel).resolve() + resolved.relative_to(ext_root) + except (OSError, ValueError): + raise ValidationError( + f"{field_name} escapes extension directory: {relative_path!r}" + ) + + return resolved + + def _materialize_config_files(self, manifest: ExtensionManifest, extension_dir: Path) -> None: + """Create config files declared in provides.config from their templates. + + Config files are only created when missing; existing files are preserved. + """ + for idx, entry in enumerate(manifest.config_files): + if not isinstance(entry, dict): + raise ValidationError( + f"Invalid provides.config entry at index {idx}: expected a mapping" + ) + + name = entry.get("name") + template = entry.get("template") + if not isinstance(name, str) or not name.strip(): + raise ValidationError( + f"Invalid provides.config[{idx}].name: expected non-empty string" + ) + if not isinstance(template, str) or not template.strip(): + raise ValidationError( + f"Invalid provides.config[{idx}].template: expected non-empty string" + ) + + name = name.strip() + template = template.strip() + + target_path = self._resolve_manifest_relative_path( + extension_dir, + name, + f"provides.config[{idx}].name", + ) + template_path = self._resolve_manifest_relative_path( + extension_dir, + template, + f"provides.config[{idx}].template", + ) + + if not template_path.is_file(): + raise ValidationError( + f"Template file not found for provides.config[{idx}]: {template!r}" + ) + + if target_path.exists(): + continue + + target_path.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(template_path, target_path) + def _get_skills_dir(self) -> Optional[Path]: """Return the active skills directory for extension skill registration. @@ -1217,6 +1299,10 @@ def install_from_directory( ignore_fn = self._load_extensionignore(source_dir) shutil.copytree(source_dir, dest_dir, ignore=ignore_fn) + # Materialize declared config files from templates (without + # overwriting existing files in the copied extension directory). + self._materialize_config_files(manifest, dest_dir) + # Register commands with AI agents registered_commands = {} if register_commands: @@ -2250,7 +2336,7 @@ class ConfigManager: Configuration layers (in order of precedence from lowest to highest): 1. Defaults (from extension.yml) 2. Project config (.specify/extensions/{ext-id}/{ext-id}-config.yml) - 3. Local config (.specify/extensions/{ext-id}/local-config.yml) - gitignored + 3. Local config (legacy local.yml + local-config.yml, with local-config.yml winning) 4. Environment variables (SPECKIT_{EXT_ID}_{KEY}) """ @@ -2310,8 +2396,11 @@ def _get_local_config(self) -> Dict[str, Any]: Returns: Local configuration dictionary """ - config_file = self.extension_dir / "local-config.yml" - return self._load_yaml_config(config_file) + # Backward compatibility: support historical local.yml while preferring + # local-config.yml when both files exist. + legacy_config = self._load_yaml_config(self.extension_dir / "local.yml") + canonical_config = self._load_yaml_config(self.extension_dir / "local-config.yml") + return self._merge_configs(legacy_config, canonical_config) def _get_env_config(self) -> Dict[str, Any]: """Get configuration from environment variables. diff --git a/src/specify_cli/integrations/claude/__init__.py b/src/specify_cli/integrations/claude/__init__.py index 4f3838b6e4..31e3cc59de 100644 --- a/src/specify_cli/integrations/claude/__init__.py +++ b/src/specify_cli/integrations/claude/__init__.py @@ -190,8 +190,38 @@ def repl(m: re.Match[str]) -> str: ) def post_process_skill_content(self, content: str) -> str: - """Inject Claude-specific frontmatter flags and hook notes.""" - updated = self._inject_frontmatter_flag(content, "user-invocable") + """Inject Claude-specific frontmatter flags, mode, and hook notes. + + Injects: + - mode: speckit. for Claude's slash command routing + - user-invocable: true (official Copilot/Claude field) + - disable-model-invocation: false (official Copilot/Claude field) + - Hook command notes for dot-to-hyphen mapping + """ + # Extract skill name to derive mode value + lines = content.splitlines(keepends=True) + dash_count = 0 + skill_name = "" + for line in lines: + stripped = line.rstrip("\n\r") + if stripped == "---": + dash_count += 1 + if dash_count == 2: + break + continue + if dash_count == 1 and stripped.startswith("name:"): + val = stripped.split(":", 1)[1].strip().strip('"').strip("'") + if val.startswith("speckit-"): + skill_name = "speckit." + val[len("speckit-"):] + else: + skill_name = val + + # Inject mode if we have a skill name + updated = content + if skill_name: + updated = self._inject_frontmatter_flag(updated, "mode", skill_name) + + updated = self._inject_frontmatter_flag(updated, "user-invocable") updated = self._inject_frontmatter_flag(updated, "disable-model-invocation", "false") updated = self._inject_hook_command_note(updated) return updated diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index c7456ce7f0..55e3b88a8c 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -254,11 +254,57 @@ def command_filename(self, template_name: str) -> str: """Copilot commands use ``.agent.md`` extension.""" return f"speckit.{template_name}.agent.md" + @staticmethod + def _inject_frontmatter_field(content: str, key: str, value: str) -> str: + """Insert ``key: value`` before the closing ``---`` if not already present. + + Handles proper YAML quoting for string values. + """ + lines = content.splitlines(keepends=True) + + # Pre-scan: bail out if already present in frontmatter + dash_count = 0 + for line in lines: + stripped = line.rstrip("\n\r") + if stripped == "---": + dash_count += 1 + if dash_count == 2: + break + continue + if dash_count == 1 and stripped.startswith(f"{key}:"): + return content + + # Inject before the closing --- of frontmatter + out: list[str] = [] + dash_count = 0 + injected = False + for line in lines: + stripped = line.rstrip("\n\r") + if stripped == "---": + dash_count += 1 + if dash_count == 2 and not injected: + if line.endswith("\r\n"): + eol = "\r\n" + elif line.endswith("\n"): + eol = "\n" + else: + eol = "" + # Quote string values that look like they need it + if value.startswith(("speckit.", "speckit-")) or " " in value: + out.append(f'{key}: "{value}"{eol}') + else: + out.append(f"{key}: {value}{eol}") + injected = True + out.append(line) + return "".join(out) + def post_process_skill_content(self, content: str) -> str: - """Inject Copilot-specific ``mode:`` field into SKILL.md frontmatter. + """Inject Copilot-specific frontmatter fields into SKILL.md. - Inserts ``mode: speckit.`` before the closing ``---`` so - Copilot can associate the skill with its agent mode. + Inserts: + - mode: speckit. for Copilot's skill dispatch + - user-invocable: true (official field, aligns with Claude) + - target: github-copilot (makes platform explicit) """ lines = content.splitlines(keepends=True) @@ -274,7 +320,9 @@ def post_process_skill_content(self, content: str) -> str: continue if dash_count == 1: if stripped.startswith("mode:"): - return content # already present + # mode already present, just add other fields if missing + updated = self._inject_frontmatter_field(content, "user-invocable", "true") + return updated if stripped.startswith("name:"): # Parse: name: "speckit-plan" → speckit.plan val = stripped.split(":", 1)[1].strip().strip('"').strip("'") @@ -287,23 +335,25 @@ def post_process_skill_content(self, content: str) -> str: if not skill_name: return content - # Inject mode: before the closing --- of frontmatter + # Inject mode and other fields before the closing --- of frontmatter out: list[str] = [] dash_count = 0 - injected = False + mode_injected = False for line in lines: stripped = line.rstrip("\n\r") if stripped == "---": dash_count += 1 - if dash_count == 2 and not injected: + if dash_count == 2 and not mode_injected: if line.endswith("\r\n"): eol = "\r\n" elif line.endswith("\n"): eol = "\n" else: eol = "" + # Inject in order: mode, then user-invocable out.append(f"mode: {skill_name}{eol}") - injected = True + out.append(f"user-invocable: true{eol}") + mode_injected = True out.append(line) return "".join(out) @@ -367,6 +417,12 @@ def _setup_default( raw, self.key, script_type, arg_placeholder, context_file=self.context_file or "", ) + + # Inject name and other Copilot-specific frontmatter + agent_name = f"speckit.{src_file.stem}" + processed = self._inject_frontmatter_field(processed, "name", agent_name) + processed = self._inject_frontmatter_field(processed, "user-invocable", "true") + dst_name = self.command_filename(src_file.stem) dst_file = self.write_file_and_record( processed, dest / dst_name, project_root, manifest diff --git a/tests/extensions/git/test_git_extension.py b/tests/extensions/git/test_git_extension.py index c4f986d177..536cefe6f6 100644 --- a/tests/extensions/git/test_git_extension.py +++ b/tests/extensions/git/test_git_extension.py @@ -837,3 +837,356 @@ def test_test_feature_branch_accepts_single_prefix(self, tmp_path: Path): text=True, ) assert result.returncode == 0 + + +# ── ConfigManager layering tests (git extension) ───────────────────────────── + + +class TestGitConfigManagerLayering: + """Unit tests for ConfigManager layering specific to the git extension. + + Covers: + 1. defaults only (from extension.yml) + 2. git-config.yml overrides defaults + 3. local-config.yml overrides project config + 4. SPECKIT_GIT_* env vars override all files + """ + + def _install_git_extension(self, project_dir: Path) -> Path: + """Install the git extension into project_dir and return the installed ext dir.""" + from specify_cli.extensions import ExtensionManager + + (project_dir / ".specify").mkdir(exist_ok=True) + manager = ExtensionManager(project_dir) + manager.install_from_directory(EXT_DIR, "0.5.0", register_commands=False) + return project_dir / ".specify" / "extensions" / "git" + + def test_defaults_only(self, tmp_path: Path): + """ConfigManager returns manifest defaults when no config files exist.""" + from specify_cli.extensions import ConfigManager + + ext_dir = self._install_git_extension(tmp_path) + # Remove any config file that install may have created + for f in ("git-config.yml", "local-config.yml"): + (ext_dir / f).unlink(missing_ok=True) + + config = ConfigManager(tmp_path, "git").get_config() + + assert config["branch_numbering"] == "sequential" + assert config["init_commit_message"] == "[Spec Kit] Initial commit" + assert config["auto_commit"]["default"] is False + + def test_project_config_overrides_defaults(self, tmp_path: Path): + """git-config.yml overrides manifest defaults.""" + from specify_cli.extensions import ConfigManager + + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").write_text( + "branch_numbering: timestamp\n" + "init_commit_message: \"[Custom] Init\"\n", + encoding="utf-8", + ) + (ext_dir / "local-config.yml").unlink(missing_ok=True) + + config = ConfigManager(tmp_path, "git").get_config() + + assert config["branch_numbering"] == "timestamp" + assert config["init_commit_message"] == "[Custom] Init" + # Unmentioned key still comes from defaults + assert "auto_commit" in config + + def test_local_config_overrides_project_config(self, tmp_path: Path): + """local-config.yml overrides git-config.yml which overrides defaults.""" + from specify_cli.extensions import ConfigManager + + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").write_text( + "branch_numbering: timestamp\n", + encoding="utf-8", + ) + (ext_dir / "local-config.yml").write_text( + "branch_numbering: sequential\n" + "init_commit_message: \"[Local] Override\"\n", + encoding="utf-8", + ) + + config = ConfigManager(tmp_path, "git").get_config() + + assert config["branch_numbering"] == "sequential" + assert config["init_commit_message"] == "[Local] Override" + + def test_env_var_creates_nested_path(self, tmp_path: Path): + """ConfigManager maps SPECKIT_GIT_A_B_C to a nested path a.b.c. + + Note: the ConfigManager splits env var suffixes on every underscore, so + keys with underscores in their name (like branch_numbering) cannot be + directly overridden via env var — the env var instead creates a *new* + nested key at that path. This test documents that design constraint. + """ + from specify_cli.extensions import ConfigManager + + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").unlink(missing_ok=True) + (ext_dir / "local-config.yml").unlink(missing_ok=True) + + # SPECKIT_GIT_AUTO_COMMIT_DEFAULT -> path ["auto", "commit", "default"] + # which does NOT match the key "auto_commit" -> it creates a new "auto" key. + env_patch = {"SPECKIT_GIT_AUTO_COMMIT_DEFAULT": "true"} + original = {k: os.environ.get(k) for k in env_patch} + try: + os.environ.update(env_patch) + config = ConfigManager(tmp_path, "git").get_config() + finally: + for k, v in original.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + + # The env var creates a new nested "auto.commit.default" path alongside "auto_commit" + assert config["auto"]["commit"]["default"] == "true" + # The original auto_commit key from defaults is unaffected + assert config["auto_commit"]["default"] is False + + def test_env_var_prefix_format(self, tmp_path: Path): + """SPECKIT_GIT_* is the correct env prefix for the git extension.""" + from specify_cli.extensions import ConfigManager + + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").unlink(missing_ok=True) + (ext_dir / "local-config.yml").unlink(missing_ok=True) + + # A single-segment suffix produces a top-level key + env_patch = {"SPECKIT_GIT_TESTKEY": "testval"} + original = {k: os.environ.get(k) for k in env_patch} + try: + os.environ.update(env_patch) + config = ConfigManager(tmp_path, "git").get_config() + finally: + for k, v in original.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + + assert config["testkey"] == "testval" + + +# ── CLI resolver integration tests (git extension) ─────────────────────────── + + +class TestGitConfigResolverCLI: + """Integration tests for `specify extension config resolve git`. + + Verifies --format json, --format env, and invalid format behaviour + against an installed git extension. + """ + + def _install_git_extension(self, project_dir: Path) -> Path: + from specify_cli.extensions import ExtensionManager + + (project_dir / ".specify").mkdir(exist_ok=True) + manager = ExtensionManager(project_dir) + manager.install_from_directory(EXT_DIR, "0.5.0", register_commands=False) + return project_dir / ".specify" / "extensions" / "git" + + def _invoke(self, project_dir: Path, *args: str, env: dict | None = None): + """Invoke the CLI via typer's test runner with project_dir as cwd.""" + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir): + return runner.invoke(app, list(args), catch_exceptions=False, env=env or {}) + + def test_resolve_json_returns_defaults(self, tmp_path: Path): + """--format json returns at least the manifest defaults.""" + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").unlink(missing_ok=True) + (ext_dir / "local-config.yml").unlink(missing_ok=True) + + result = self._invoke(tmp_path, "extension", "config", "resolve", "git", "--format", "json") + assert result.exit_code == 0, result.output + import json as _json + data = _json.loads(result.output) + assert data["branch_numbering"] == "sequential" + assert data["init_commit_message"] == "[Spec Kit] Initial commit" + assert data["auto_commit"]["default"] is False + + def test_resolve_json_project_config_overrides(self, tmp_path: Path): + """--format json reflects project config overrides on top of defaults.""" + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").write_text( + "branch_numbering: timestamp\n", + encoding="utf-8", + ) + (ext_dir / "local-config.yml").unlink(missing_ok=True) + + result = self._invoke(tmp_path, "extension", "config", "resolve", "git", "--format", "json") + assert result.exit_code == 0, result.output + import json as _json + data = _json.loads(result.output) + assert data["branch_numbering"] == "timestamp" + + def test_resolve_env_prefix_normalisation(self, tmp_path: Path): + """--format env emits GIT_CFG_BRANCH_NUMBERING with correct prefix.""" + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").unlink(missing_ok=True) + (ext_dir / "local-config.yml").unlink(missing_ok=True) + + result = self._invoke( + tmp_path, + "extension", "config", "resolve", "git", + "--format", "env", + "--prefix", "GIT_CFG_", + ) + assert result.exit_code == 0, result.output + assert "GIT_CFG_BRANCH_NUMBERING=" in result.output + assert "GIT_CFG_INIT_COMMIT_MESSAGE=" in result.output + + def test_resolve_env_local_config_reflected(self, tmp_path: Path): + """--format env reflects local-config.yml override.""" + ext_dir = self._install_git_extension(tmp_path) + (ext_dir / "git-config.yml").write_text( + "branch_numbering: sequential\n", + encoding="utf-8", + ) + (ext_dir / "local-config.yml").write_text( + "branch_numbering: timestamp\n", + encoding="utf-8", + ) + + result = self._invoke( + tmp_path, + "extension", "config", "resolve", "git", + "--format", "env", + "--prefix", "GIT_CFG_", + ) + assert result.exit_code == 0, result.output + assert "GIT_CFG_BRANCH_NUMBERING=timestamp" in result.output + + def test_resolve_invalid_format_exits_nonzero(self, tmp_path: Path): + """--format yaml is rejected with exit code 1.""" + self._install_git_extension(tmp_path) + + result = self._invoke( + tmp_path, + "extension", "config", "resolve", "git", + "--format", "yaml", + ) + assert result.exit_code != 0 + + +# ── initialize-repo.sh env-var override tests ──────────────────────────────── + + +@requires_bash +class TestInitializeRepoEnvOverride: + """initialize-repo.sh reads GIT_CFG_INIT_COMMIT_MESSAGE before falling back to YAML.""" + + def test_env_var_overrides_config_file(self, tmp_path: Path): + """GIT_CFG_INIT_COMMIT_MESSAGE takes priority over git-config.yml.""" + project = _setup_project(tmp_path, git=False) + _write_config(project, 'init_commit_message: "From config file"\n') + + result = _run_bash( + "initialize-repo.sh", project, + env_extra={"GIT_CFG_INIT_COMMIT_MESSAGE": "From env var"}, + ) + assert result.returncode == 0, result.stderr + + log = subprocess.run( + ["git", "log", "--oneline", "-1"], + cwd=project, capture_output=True, text=True, + ) + assert "From env var" in log.stdout + + def test_env_var_overrides_default(self, tmp_path: Path): + """GIT_CFG_INIT_COMMIT_MESSAGE overrides the hardcoded default when no config file.""" + project = _setup_project(tmp_path, git=False) + config = project / ".specify" / "extensions" / "git" / "git-config.yml" + config.unlink(missing_ok=True) + + result = _run_bash( + "initialize-repo.sh", project, + env_extra={"GIT_CFG_INIT_COMMIT_MESSAGE": "Env-only message"}, + ) + assert result.returncode == 0, result.stderr + + log = subprocess.run( + ["git", "log", "--oneline", "-1"], + cwd=project, capture_output=True, text=True, + ) + assert "Env-only message" in log.stdout + + +# ── auto-commit.sh env-var override tests ──────────────────────────────────── + + +@requires_bash +class TestAutoCommitEnvOverride: + """auto-commit.sh reads GIT_CFG_AUTO_COMMIT_* env vars before falling back to YAML.""" + + def test_env_enabled_overrides_disabled_config(self, tmp_path: Path): + """GIT_CFG_AUTO_COMMIT_AFTER_SPECIFY_ENABLED=true commits even when config says false.""" + project = _setup_project(tmp_path) + _write_config(project, ( + "auto_commit:\n" + " default: false\n" + " after_specify:\n" + " enabled: false\n" + )) + (project / "new-file.txt").write_text("content") + + result = _run_bash( + "auto-commit.sh", project, "after_specify", + env_extra={ + "GIT_CFG_AUTO_COMMIT_DEFAULT": "false", + "GIT_CFG_AUTO_COMMIT_AFTER_SPECIFY_ENABLED": "true", + "GIT_CFG_AUTO_COMMIT_AFTER_SPECIFY_MESSAGE": "Env-driven commit", + }, + ) + assert result.returncode == 0, result.stderr + + log = subprocess.run( + ["git", "log", "--oneline", "-1"], + cwd=project, capture_output=True, text=True, + ) + assert "Env-driven commit" in log.stdout + + def test_env_default_true_with_no_event_key(self, tmp_path: Path): + """GIT_CFG_AUTO_COMMIT_DEFAULT=true triggers commit when no per-event env var.""" + project = _setup_project(tmp_path) + # No config file — purely env-driven + config = project / ".specify" / "extensions" / "git" / "git-config.yml" + config.unlink(missing_ok=True) + (project / "new-file.txt").write_text("content") + + result = _run_bash( + "auto-commit.sh", project, "after_tasks", + env_extra={"GIT_CFG_AUTO_COMMIT_DEFAULT": "true"}, + ) + assert result.returncode == 0, result.stderr + assert "[OK] Changes committed" in result.stderr + + def test_env_disabled_prevents_commit(self, tmp_path: Path): + """GIT_CFG_AUTO_COMMIT_AFTER_SPECIFY_ENABLED=false suppresses commit even when default=true.""" + project = _setup_project(tmp_path) + _write_config(project, "auto_commit:\n default: true\n") + (project / "new-file.txt").write_text("content") + + result = _run_bash( + "auto-commit.sh", project, "after_specify", + env_extra={ + "GIT_CFG_AUTO_COMMIT_DEFAULT": "true", + "GIT_CFG_AUTO_COMMIT_AFTER_SPECIFY_ENABLED": "false", + }, + ) + assert result.returncode == 0, result.stderr + # Verify no new commit was made (still only the seed commit) + log = subprocess.run( + ["git", "log", "--oneline"], + cwd=project, capture_output=True, text=True, + ) + assert log.stdout.strip().count("\n") == 0 diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 7732d57300..6f6c677b61 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -258,8 +258,8 @@ def test_shared_infra_skip_warning_displayed(self, tmp_path, capsys): captured = capsys.readouterr() assert "already exist and were not updated" in captured.out assert "specify init --here --force" in captured.out - # Rich may wrap long lines; normalize whitespace for the second command - normalized = " ".join(captured.out.split()) + # Rich may inject styling/wrapping; normalize before substring checks + normalized = _normalize_cli_output(captured.out) assert "specify integration upgrade --force" in normalized def test_shared_infra_warns_when_manifest_cannot_be_loaded(self, tmp_path, capsys): diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 750bbb6efa..4e258a2de0 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -7,6 +7,7 @@ from typer.testing import CliRunner from specify_cli import app +from tests.conftest import strip_ansi runner = CliRunner() @@ -49,7 +50,10 @@ def _write_invalid_manifest(project, key): def _integration_list_row_cells(output: str, key: str) -> list[str]: - row = next(line for line in output.splitlines() if line.startswith(f"│ {key}")) + normalized_output = strip_ansi(output) + row = next( + line for line in normalized_output.splitlines() if line.lstrip().startswith(f"│ {key}") + ) return [cell.strip() for cell in row.split("│")[1:-1]] @@ -161,7 +165,7 @@ def test_install_already_installed(self, tmp_path): os.chdir(old_cwd) assert result.exit_code == 0 assert "already installed" in result.output - normalized = " ".join(result.output.split()) + normalized = " ".join(strip_ansi(result.output).split()) assert "specify integration upgrade copilot" in normalized assert "specify integration uninstall copilot" in normalized @@ -174,9 +178,10 @@ def test_install_different_when_one_exists(self, tmp_path): finally: os.chdir(old_cwd) assert result.exit_code != 0 - assert "Installed integrations: copilot" in result.output - assert "Default integration: copilot" in result.output - assert "--force" in result.output + normalized = " ".join(strip_ansi(result.output).split()) + assert "Installed integrations: copilot" in normalized + assert "Default integration: copilot" in normalized + assert "--force" in normalized def test_install_multi_safe_integration(self, tmp_path): project = _init_project(tmp_path, "claude") diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 147dc99f3c..9b069f912d 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -769,6 +769,90 @@ def test_install_from_directory(self, extension_dir, project_dir): assert (ext_dir / "extension.yml").exists() assert (ext_dir / "commands" / "hello.md").exists() + def test_install_materializes_config_file_from_template(self, temp_dir, project_dir): + """Install should create provides.config target files from templates.""" + import yaml + + ext_dir = temp_dir / "cfg-ext" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") + (ext_dir / "config-template.yml").write_text("alpha: 1\nbeta:\n nested: true\n") + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.cfg-ext.cmd", + "file": "commands/cmd.md", + } + ], + "config": [ + { + "name": "cfg-ext-config.yml", + "template": "config-template.yml", + "required": False, + } + ], + }, + } + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + manager = ExtensionManager(project_dir) + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + installed_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + generated = installed_dir / "cfg-ext-config.yml" + assert generated.exists() + assert generated.read_text() == (installed_dir / "config-template.yml").read_text() + + def test_install_config_template_missing_raises_validation_error(self, temp_dir, project_dir): + """Install should fail when provides.config references a missing template file.""" + import yaml + + ext_dir = temp_dir / "missing-template" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "missing-template", + "name": "Missing Template", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.missing-template.cmd", + "file": "commands/cmd.md", + } + ], + "config": [ + { + "name": "missing-template-config.yml", + "template": "does-not-exist.yml", + } + ], + }, + } + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + manager = ExtensionManager(project_dir) + with pytest.raises(ValidationError, match="Template file not found"): + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + def test_install_duplicate(self, extension_dir, project_dir): """Test installing already installed extension.""" manager = ExtensionManager(project_dir) @@ -3765,6 +3849,465 @@ def test_add_bundled_extension_not_found_gives_clear_error(self, tmp_path): assert "reinstall" in result.output.lower() +class TestExtensionConfigResolveCLI: + """CLI integration tests for extension config resolve.""" + + @staticmethod + def _install_configured_extension(project_dir: Path, tmp_path: Path) -> None: + import yaml + + ext_dir = tmp_path / "cfg-ext" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") + (ext_dir / "config-template.yml").write_text( + "depth:\n level: signatures\nextra:\n mode: from-template\n" + ) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.cfg-ext.cmd", + "file": "commands/cmd.md", + } + ], + "config": [ + { + "name": "cfg-ext-config.yml", + "template": "config-template.yml", + } + ], + }, + "config": { + "defaults": { + "depth": {"level": "metadata", "max_lines": 100}, + "feature": {"enabled": False}, + } + }, + } + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + manager = ExtensionManager(project_dir) + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + installed_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + (installed_dir / "local-config.yml").write_text( + "depth:\n max_lines: 250\nlocal:\n only: true\n" + ) + + def test_resolve_json_outputs_layered_config(self, tmp_path): + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".specify" / "extensions").mkdir(parents=True) + + self._install_configured_extension(project_dir, tmp_path) + + env = { + "SPECKIT_CFG_EXT_DEPTH_LEVEL": "full", + } + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, + ["extension", "config", "resolve", "cfg-ext", "--format", "json"], + env=env, + catch_exceptions=False, + ) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["depth"]["level"] == "full" + assert payload["depth"]["max_lines"] == 250 + assert payload["extra"]["mode"] == "from-template" + assert payload["feature"]["enabled"] is False + assert payload["local"]["only"] is True + + def test_resolve_env_outputs_flat_assignments(self, tmp_path): + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".specify" / "extensions").mkdir(parents=True) + + self._install_configured_extension(project_dir, tmp_path) + + env = { + "SPECKIT_CFG_EXT_DEPTH_LEVEL": "full", + } + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, + [ + "extension", + "config", + "resolve", + "cfg-ext", + "--format", + "env", + "--prefix", + "REVENGE_CFG", + ], + env=env, + catch_exceptions=False, + ) + + assert result.exit_code == 0 + output_lines = result.output.splitlines() + assert "REVENGE_CFG_DEPTH_LEVEL=full" in output_lines + assert "REVENGE_CFG_DEPTH_MAX_LINES=250" in output_lines + assert "REVENGE_CFG_FEATURE_ENABLED=false" in output_lines + assert "REVENGE_CFG_LOCAL_ONLY=true" in output_lines + + def test_resolve_json_prefers_local_over_project_file(self, tmp_path): + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".specify" / "extensions").mkdir(parents=True) + + self._install_configured_extension(project_dir, tmp_path) + + installed_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + (installed_dir / "cfg-ext-config.yml").write_text( + "depth:\n" + " level: logic\n" + " max_lines: 220\n" + "feature:\n" + " enabled: true\n" + "project:\n" + " from_file: true\n" + ) + (installed_dir / "local-config.yml").write_text( + "depth:\n" + " max_lines: 260\n" + "local:\n" + " from_file: true\n" + ) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, + ["extension", "config", "resolve", "cfg-ext", "--format", "json"], + catch_exceptions=False, + ) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["depth"]["level"] == "logic" + assert payload["depth"]["max_lines"] == 260 + assert payload["feature"]["enabled"] is True + assert payload["project"]["from_file"] is True + assert payload["local"]["from_file"] is True + + def test_resolve_by_display_name(self, tmp_path): + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".specify" / "extensions").mkdir(parents=True) + + self._install_configured_extension(project_dir, tmp_path) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, + ["extension", "config", "resolve", "Config Extension", "--format", "json"], + catch_exceptions=False, + ) + + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["depth"]["max_lines"] == 250 + assert payload["local"]["only"] is True + + def test_resolve_invalid_format_fails_with_clear_error(self, tmp_path): + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + (project_dir / ".specify" / "extensions").mkdir(parents=True) + + self._install_configured_extension(project_dir, tmp_path) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, + ["extension", "config", "resolve", "cfg-ext", "--format", "yaml"], + catch_exceptions=False, + ) + + assert result.exit_code == 1 + output = strip_ansi(result.output) + assert "--format must be one of: json, env" in output + + +class TestConfigManagerLayering: + """Unit tests for ConfigManager file-layer precedence.""" + + def test_get_config_uses_extension_config_and_local_override(self, tmp_path): + """local-config.yml should override -config.yml, which overrides defaults.""" + import yaml + from specify_cli.extensions import ConfigManager + + project_dir = tmp_path / "project" + extension_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + extension_dir.mkdir(parents=True) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": []}, + "config": { + "defaults": { + "depth": {"level": "metadata", "max_lines": 100}, + "feature": {"enabled": False}, + } + }, + } + (extension_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + # Project config layer (-config.yml) + (extension_dir / "cfg-ext-config.yml").write_text( + "depth:\n" + " level: signatures\n" + " max_lines: 200\n" + "project:\n" + " from_file: true\n" + ) + + # Local config layer (local-config.yml) + (extension_dir / "local-config.yml").write_text( + "depth:\n" + " max_lines: 300\n" + "local:\n" + " from_file: true\n" + ) + + config = ConfigManager(project_dir, "cfg-ext").get_config() + + # defaults -> project config -> local config + assert config["depth"]["level"] == "signatures" + assert config["depth"]["max_lines"] == 300 + assert config["feature"]["enabled"] is False + assert config["project"]["from_file"] is True + assert config["local"]["from_file"] is True + + def test_get_config_without_local_uses_extension_config(self, tmp_path): + """When local-config.yml is absent, -config.yml should still override defaults.""" + import yaml + from specify_cli.extensions import ConfigManager + + project_dir = tmp_path / "project" + extension_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + extension_dir.mkdir(parents=True) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": []}, + "config": { + "defaults": { + "depth": {"level": "metadata", "max_lines": 100}, + } + }, + } + (extension_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + (extension_dir / "cfg-ext-config.yml").write_text( + "depth:\n" + " level: logic\n" + " max_lines: 220\n" + ) + + config = ConfigManager(project_dir, "cfg-ext").get_config() + + assert config["depth"]["level"] == "logic" + assert config["depth"]["max_lines"] == 220 + + def test_get_config_legacy_local_yml_overrides_extension_config(self, tmp_path): + """Legacy local.yml should still act as local override when local-config.yml is absent.""" + import yaml + from specify_cli.extensions import ConfigManager + + project_dir = tmp_path / "project" + extension_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + extension_dir.mkdir(parents=True) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": []}, + "config": { + "defaults": { + "depth": {"level": "metadata", "max_lines": 100}, + } + }, + } + (extension_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + (extension_dir / "cfg-ext-config.yml").write_text( + "depth:\n" + " level: signatures\n" + " max_lines: 200\n" + ) + (extension_dir / "local.yml").write_text( + "depth:\n" + " max_lines: 350\n" + "legacy_local:\n" + " used: true\n" + ) + + config = ConfigManager(project_dir, "cfg-ext").get_config() + + assert config["depth"]["level"] == "signatures" + assert config["depth"]["max_lines"] == 350 + assert config["legacy_local"]["used"] is True + + def test_get_config_prefers_local_config_over_legacy_local_yml(self, tmp_path): + """When both local files exist, local-config.yml should win over local.yml.""" + import yaml + from specify_cli.extensions import ConfigManager + + project_dir = tmp_path / "project" + extension_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + extension_dir.mkdir(parents=True) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": []}, + "config": { + "defaults": { + "depth": {"level": "metadata", "max_lines": 100}, + } + }, + } + (extension_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + (extension_dir / "cfg-ext-config.yml").write_text( + "depth:\n" + " max_lines: 200\n" + ) + (extension_dir / "local.yml").write_text( + "depth:\n" + " max_lines: 350\n" + ) + (extension_dir / "local-config.yml").write_text( + "depth:\n" + " max_lines: 400\n" + ) + + config = ConfigManager(project_dir, "cfg-ext").get_config() + + assert config["depth"]["max_lines"] == 400 + + def test_get_config_env_overrides_project_and_local(self, tmp_path, monkeypatch): + """Environment values should override both project and local file layers.""" + import yaml + from specify_cli.extensions import ConfigManager + + project_dir = tmp_path / "project" + extension_dir = project_dir / ".specify" / "extensions" / "cfg-ext" + extension_dir.mkdir(parents=True) + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cfg-ext", + "name": "Config Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"commands": []}, + "config": { + "defaults": { + "depth": {"level": "metadata", "max_lines": 100}, + } + }, + } + (extension_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + + (extension_dir / "cfg-ext-config.yml").write_text( + "depth:\n" + " level: signatures\n" + " max_lines: 200\n" + ) + (extension_dir / "local-config.yml").write_text( + "depth:\n" + " max_lines: 300\n" + ) + + monkeypatch.setenv("SPECKIT_CFG_EXT_DEPTH_LEVEL", "full") + + config = ConfigManager(project_dir, "cfg-ext").get_config() + + # env -> local -> project -> defaults + assert config["depth"]["level"] == "full" + assert config["depth"]["max_lines"] == 300 + + class TestDownloadExtensionBundled: """Tests for download_extension handling of bundled extensions.""" From e932dd0e7b0c28fcfb9f8a00afc221c84261ca1b Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 9 May 2026 13:26:46 +0200 Subject: [PATCH 4/5] feat: add seamless extension update from dev directory with --dev mode - Add update_from_directory() method to ExtensionManager for in-place updates - Add --dev and --dev-path options to extension update command - Preserve extension state (enabled, priority, installed_at) during updates - Maintain user configuration files (*-config.yml) through updates - Implement atomic rollback on failure - Avoid remove/reinstall cycle for faster development iteration --- src/specify_cli/__init__.py | 56 ++++++++++++++- src/specify_cli/extensions.py | 125 ++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 58404a3658..25ae9b50d0 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -4799,6 +4799,8 @@ def _print_extension_info(ext_info: dict, manager): @extension_app.command("update") def extension_update( extension: str = typer.Argument(None, help="Extension ID or name to update (or all)"), + dev: bool = typer.Option(False, "--dev", help="Update from local development directory"), + dev_path: Optional[str] = typer.Option(None, "--dev-path", help="Path to development directory (requires --dev)"), ): """Update extension(s) to latest version.""" from .extensions import ( @@ -4815,10 +4817,62 @@ def extension_update( project_root = _require_specify_project() manager = ExtensionManager(project_root) - catalog = ExtensionCatalog(project_root) speckit_version = get_speckit_version() try: + # Handle --dev mode (update from local directory) + if dev: + if not extension: + console.print("[red]Error:[/red] Extension ID required when using --dev") + raise typer.Exit(1) + + installed = manager.list_installed() + extension_id, ext_display_name = _resolve_installed_extension(extension, installed, "update") + + # Use provided path or prompt for it + if dev_path: + source_path = Path(dev_path).expanduser().resolve() + else: + console.print(f"[cyan]Updating {ext_display_name}...[/cyan]") + console.print("Enter the path to the development directory:") + dev_path_input = typer.prompt("Path") + source_path = Path(dev_path_input).expanduser().resolve() + + # Validate source directory + if not source_path.exists(): + console.print(f"[red]Error:[/red] Directory not found: {source_path}") + raise typer.Exit(1) + + if not (source_path / "extension.yml").exists(): + console.print(f"[red]Error:[/red] No extension.yml found in {source_path}") + raise typer.Exit(1) + + # Perform in-place update from dev directory + with console.status(f"[cyan]Updating {ext_display_name} from {source_path}...[/cyan]"): + try: + new_manifest = manager.update_from_directory( + source_path, + speckit_version, + extension_id + ) + + console.print(f"\n[green]✓[/green] Updated {ext_display_name} to v{new_manifest.version}") + console.print("\n[bold cyan]Commands:[/bold cyan]") + for cmd in new_manifest.commands: + console.print(f" • {cmd['name']} - {cmd.get('description', '')}") + + except ExtensionError as e: + console.print(f"\n[red]Error:[/red] {e}") + raise typer.Exit(1) + except ValidationError as e: + console.print(f"\n[red]Validation Error:[/red] {e}") + raise typer.Exit(1) + + raise typer.Exit(0) + + # Non-dev mode: update from catalog + catalog = ExtensionCatalog(project_root) + # Get list of extensions to update installed = manager.list_installed() if extension: diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 9b412895db..b8c9b9e310 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1333,6 +1333,131 @@ def install_from_directory( return manifest + def update_from_directory( + self, + source_dir: Path, + speckit_version: str, + extension_id: str, + ) -> ExtensionManifest: + """Update an installed extension from a local directory (in-place). + + Performs a seamless update without remove/reinstall cycle. Preserves + extension state (enabled, priority, installed_at) and configuration files. + + Args: + source_dir: Path to extension directory + speckit_version: Current spec-kit version + extension_id: ID of extension to update + + Returns: + Updated extension manifest + + Raises: + ExtensionError: If extension is not installed + ValidationError: If manifest is invalid + CompatibilityError: If extension is incompatible + """ + # Verify extension is installed + if not self.registry.is_installed(extension_id): + raise ExtensionError( + f"Extension '{extension_id}' is not installed. " + f"Use 'specify extension add --dev {source_dir}' to install it." + ) + + # Load and validate new manifest + manifest_path = source_dir / "extension.yml" + new_manifest = ExtensionManifest(manifest_path) + + # Verify IDs match + if new_manifest.id != extension_id: + raise ValidationError( + f"Extension ID mismatch: expected '{extension_id}', got '{new_manifest.id}'" + ) + + # Check compatibility + self.check_compatibility(new_manifest, speckit_version) + + # Get old metadata before any changes + old_metadata = self.registry.get(extension_id) + if old_metadata is None or not isinstance(old_metadata, dict): + raise ExtensionError( + f"Registry entry for '{extension_id}' is corrupted" + ) + + dest_dir = self.extensions_dir / extension_id + registrar = CommandRegistrar() + + try: + # 1. Unregister old commands + old_registered_commands = old_metadata.get("registered_commands", {}) + if old_registered_commands: + registrar.unregister_commands(old_registered_commands, self.project_root) + + # 2. Backup config files before updating extension directory + config_backups = {} + if dest_dir.exists(): + for cfg_file in dest_dir.glob("*.yml"): + if cfg_file.name == "extension.yml": + continue + config_backups[cfg_file.name] = cfg_file.read_text(encoding="utf-8") + + # 3. Clear extension directory (but preserve the directory itself) + if dest_dir.exists(): + for item in dest_dir.iterdir(): + if item.is_dir(): + shutil.rmtree(item) + else: + item.unlink() + + # 4. Copy new extension files + ignore_fn = self._load_extensionignore(source_dir) + for item in source_dir.iterdir(): + if ignore_fn and ignore_fn((str(source_dir), [item.name])): + continue + if item.is_dir(): + shutil.copytree(item, dest_dir / item.name) + else: + shutil.copy2(item, dest_dir / item.name) + + # 5. Materialize new config files from templates + self._materialize_config_files(new_manifest, dest_dir) + + # 6. Restore old config files (user configs always win) + for cfg_name, cfg_content in config_backups.items(): + (dest_dir / cfg_name).write_text(cfg_content, encoding="utf-8") + + # 7. Re-register commands + registered_commands = registrar.register_commands_for_all_agents( + new_manifest, dest_dir, self.project_root + ) + + # 8. Update registry while preserving state + new_metadata = { + "version": new_manifest.version, + "source": old_metadata.get("source", "local"), + "manifest_hash": new_manifest.get_hash(), + "enabled": old_metadata.get("enabled", True), + "priority": old_metadata.get("priority", 10), + "registered_commands": registered_commands, + "registered_skills": old_metadata.get("registered_skills", []), + } + + # Preserve original installation timestamp + if "installed_at" in old_metadata: + new_metadata["installed_at"] = old_metadata["installed_at"] + + self.registry.restore(extension_id, new_metadata) + + return new_manifest + + except Exception: + # Attempt rollback by restoring old metadata + try: + self.registry.restore(extension_id, old_metadata) + except Exception: + pass # Registry restore failed, but we tried + raise + def install_from_zip( self, zip_path: Path, From 2d97920e1b19f779cc76d7c909be2f2187f83a01 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 9 May 2026 13:33:02 +0200 Subject: [PATCH 5/5] fix: extension update --dev command signature and ignore function bug - Change CLI signature from '--dev --dev-path ' to '--dev ' for consistency with 'add --dev' - Fix TypeError in update_from_directory: ignore_fn was being called incorrectly - ignore_fn returns a set of names to ignore, check membership instead of calling with tuple - Clear and recreate extension directory atomically instead of partial deletion --- src/specify_cli/__init__.py | 12 ++---------- src/specify_cli/extensions.py | 14 ++++++-------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 25ae9b50d0..9229101dc8 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -4799,8 +4799,7 @@ def _print_extension_info(ext_info: dict, manager): @extension_app.command("update") def extension_update( extension: str = typer.Argument(None, help="Extension ID or name to update (or all)"), - dev: bool = typer.Option(False, "--dev", help="Update from local development directory"), - dev_path: Optional[str] = typer.Option(None, "--dev-path", help="Path to development directory (requires --dev)"), + dev: Optional[str] = typer.Option(None, "--dev", help="Update from local development directory (path)"), ): """Update extension(s) to latest version.""" from .extensions import ( @@ -4829,14 +4828,7 @@ def extension_update( installed = manager.list_installed() extension_id, ext_display_name = _resolve_installed_extension(extension, installed, "update") - # Use provided path or prompt for it - if dev_path: - source_path = Path(dev_path).expanduser().resolve() - else: - console.print(f"[cyan]Updating {ext_display_name}...[/cyan]") - console.print("Enter the path to the development directory:") - dev_path_input = typer.prompt("Path") - source_path = Path(dev_path_input).expanduser().resolve() + source_path = Path(dev).expanduser().resolve() # Validate source directory if not source_path.exists(): diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index b8c9b9e310..06f4c0942b 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1401,18 +1401,16 @@ def update_from_directory( continue config_backups[cfg_file.name] = cfg_file.read_text(encoding="utf-8") - # 3. Clear extension directory (but preserve the directory itself) + # 3. Clear extension directory and copy new files if dest_dir.exists(): - for item in dest_dir.iterdir(): - if item.is_dir(): - shutil.rmtree(item) - else: - item.unlink() + shutil.rmtree(dest_dir) + dest_dir.mkdir(parents=True, exist_ok=True) - # 4. Copy new extension files + # 4. Copy new extension files using the same ignore logic as install ignore_fn = self._load_extensionignore(source_dir) for item in source_dir.iterdir(): - if ignore_fn and ignore_fn((str(source_dir), [item.name])): + # Check if this item should be ignored + if ignore_fn and item.name in ignore_fn(str(source_dir), [item.name]): continue if item.is_dir(): shutil.copytree(item, dest_dir / item.name)