diff --git a/src/skilz/commands/read_cmd.py b/src/skilz/commands/read_cmd.py index 9d4384d..fcf6253 100644 --- a/src/skilz/commands/read_cmd.py +++ b/src/skilz/commands/read_cmd.py @@ -8,8 +8,19 @@ import argparse import sys +from skilz.agents import ExtendedAgentType from skilz.scanner import find_installed_skill +# Fallback search order when --agent is not specified +# Searches directories in order until skill is found +FALLBACK_SEARCH_ORDER: list[ExtendedAgentType] = [ + "claude", # .claude/skills/ + "universal", # .skilz/skills/ + "gemini", # .skilz/skills/ (gemini uses .skilz for project) + "opencode", # .opencode/skill/ or .skilz/skills/ + "codex", # .codex/skills/ +] + def cmd_read(args: argparse.Namespace) -> int: """ @@ -29,21 +40,44 @@ def cmd_read(args: argparse.Namespace) -> int: agent = getattr(args, "agent", None) project_level: bool = getattr(args, "project", False) - # Find the skill - skill = find_installed_skill( - skill_id_or_name=skill_name, - agent=agent, - project_level=project_level, - ) + skill = None - if skill is None: + if agent: + # Agent specified - search only that agent's directories + skill = find_installed_skill( + skill_id_or_name=skill_name, + agent=agent, + project_level=project_level, + ) # Try project-level if user-level not found - if not project_level: + if skill is None and not project_level: skill = find_installed_skill( skill_id_or_name=skill_name, agent=agent, project_level=True, ) + else: + # No agent specified - use fallback search across multiple directories + # First try user-level for agents that support it + for fallback_agent in FALLBACK_SEARCH_ORDER: + skill = find_installed_skill( + skill_id_or_name=skill_name, + agent=fallback_agent, + project_level=False, # Try user-level first + ) + if skill is not None: + break + + # If not found at user-level, try project-level + if skill is None: + for fallback_agent in FALLBACK_SEARCH_ORDER: + skill = find_installed_skill( + skill_id_or_name=skill_name, + agent=fallback_agent, + project_level=True, + ) + if skill is not None: + break if skill is None: print(f"Error: Skill '{skill_name}' not found.", file=sys.stderr) diff --git a/src/skilz/config_sync.py b/src/skilz/config_sync.py index 81fde79..6d603c4 100644 --- a/src/skilz/config_sync.py +++ b/src/skilz/config_sync.py @@ -18,16 +18,50 @@ SECTION_START = "" SECTION_END = "" -# Usage instructions template -USAGE_TEMPLATE = """ + +def _generate_usage_template( + agent_name: str, + native_support: str, + force_extended: bool = False, +) -> str: + """Generate agent-specific usage template. + + Args: + agent_name: The agent identifier (e.g., "claude", "gemini") + native_support: The native_skill_support value ("all", "home", "none") + force_extended: If True, always include extended step-by-step instructions + (used when --force-config is specified) + + Returns: + Formatted XML usage block with correct invocation command + """ + # Claude with native "all" support doesn't need --agent flag + if agent_name == "claude" and native_support == "all": + invocation = 'Bash("skilz read ")' + else: + invocation = f'Bash("skilz read --agent {agent_name}")' + + # Extended instructions for agents without native support OR when forced + if native_support == "none" or force_extended: + extra_steps = """ +Step-by-step process: +1. Identify a skill from that matches the user's request +2. Run the command above to load the skill's SKILL.md content +3. Follow the instructions in the loaded skill content +4. Skills may include bundled scripts, templates, and references +""" + else: + extra_steps = "" + + return f""" When users ask you to perform tasks, check if any of the available skills below can help complete the task more effectively. How to use skills: -- Invoke: Bash("skilz read ") +- Invoke: {invocation} - The skill content will load with detailed instructions - Base directory provided in output for resolving bundled resources - +{extra_steps} Usage notes: - Only use skills listed in below - Do not invoke a skill that is already loaded in your context @@ -189,12 +223,18 @@ def format_skill_element( def _build_skills_section( skills: list[SkillReference], project_dir: Path | None = None, + agent_name: str = "claude", + native_support: str = "all", + force_extended: bool = False, ) -> str: """Build the complete skills section content following agentskills.io standard. Args: skills: List of skills to include. project_dir: Project directory for calculating relative paths. + agent_name: The agent identifier for generating correct invocation. + native_support: The agent's native_skill_support level. + force_extended: If True, include extended instructions (--force-config). Returns: Complete section content including markers. @@ -204,13 +244,14 @@ def _build_skills_section( skill_elements.append(format_skill_element(skill, project_dir)) skills_xml = "\n\n".join(skill_elements) + usage_template = _generate_usage_template(agent_name, native_support, force_extended) return f""" ## Available Skills {SECTION_START} -{USAGE_TEMPLATE} +{usage_template} @@ -258,6 +299,9 @@ def _merge_skill_into_section( existing_section: str, new_skill: SkillReference, project_dir: Path | None = None, + agent_name: str = "claude", + native_support: str = "all", + force_extended: bool = False, ) -> str: """Merge a new skill into an existing skills section. @@ -265,6 +309,9 @@ def _merge_skill_into_section( existing_section: The existing section content. new_skill: The new skill to add. project_dir: Project directory for calculating relative paths. + agent_name: The agent identifier for generating correct invocation. + native_support: The agent's native_skill_support level. + force_extended: If True, include extended instructions (--force-config). Returns: Updated section content. @@ -299,13 +346,14 @@ def _get_skill_name(elem: str) -> str: skill_elements.sort(key=_get_skill_name) skills_xml = "\n\n".join(skill_elements) + usage_template = _generate_usage_template(agent_name, native_support, force_extended) return f""" ## Available Skills {SECTION_START} -{USAGE_TEMPLATE} +{usage_template} @@ -323,6 +371,7 @@ def update_config_file( agent_config: AgentConfig, project_dir: Path | None = None, create_if_missing: bool = True, + force_extended: bool = False, ) -> ConfigSyncResult: """Update a config file with a skill reference. @@ -334,6 +383,7 @@ def update_config_file( agent_config: The agent configuration. project_dir: Project directory for calculating relative paths. create_if_missing: If True, create the config file if it doesn't exist. + force_extended: If True, include extended instructions (--force-config). Returns: ConfigSyncResult indicating what happened. @@ -375,7 +425,14 @@ def update_config_file( if system_start and system_end: # Update existing section existing_section = content[system_start.start() : system_end.end()] - new_section = _merge_skill_into_section(existing_section, skill, project_dir) + new_section = _merge_skill_into_section( + existing_section, + skill, + project_dir, + agent_name=agent_config.name, + native_support=agent_config.native_skill_support, + force_extended=force_extended, + ) new_content = ( content[: system_start.start()] + new_section + content[system_end.end() :] @@ -383,7 +440,13 @@ def update_config_file( updated = True else: # Append new section at end - new_section = _build_skills_section([skill], project_dir) + new_section = _build_skills_section( + [skill], + project_dir, + agent_name=agent_config.name, + native_support=agent_config.native_skill_support, + force_extended=force_extended, + ) if not content.endswith("\n"): content += "\n" new_content = content + "\n" + new_section + "\n" @@ -418,6 +481,7 @@ def sync_skill_to_configs( agent: str | None = None, verbose: bool = False, target_files: tuple[str, ...] | None = None, # SKILZ-50: Custom file override + force_extended: bool = False, ) -> list[ConfigSyncResult]: """Sync a skill reference to all relevant config files. @@ -430,6 +494,7 @@ def sync_skill_to_configs( target_files: Optional tuple of config file names to update (e.g., ("GEMINI.md",)). If provided, overrides auto-detection and only updates these files. The agent parameter is ignored when target_files is specified. + force_extended: If True, include extended instructions (--force-config). Returns: List of ConfigSyncResult for each config file processed. @@ -484,6 +549,7 @@ def sync_skill_to_configs( agent_config=agent_config, project_dir=project_dir, create_if_missing=(agent is not None), # Only create if agent specified + force_extended=force_extended, ) results.append(result) diff --git a/src/skilz/data/LOAD_SKILLS.md b/src/skilz/data/LOAD_SKILLS.md new file mode 100644 index 0000000..fa4b43e --- /dev/null +++ b/src/skilz/data/LOAD_SKILLS.md @@ -0,0 +1,84 @@ +# How to Use Skills + +This guide explains how AI coding agents can discover and use skills installed via the skilz CLI. + +## What Are Skills? + +Skills are reusable instruction sets that extend an AI agent's capabilities. Each skill contains: +- **SKILL.md** - The main instruction file with guidance, examples, and commands +- **Bundled resources** - Scripts, templates, reference docs, and other files + +## Discovering Available Skills + +Skills are listed in your agent's config file under ``: + +```xml + + + plantuml + Generate PlantUML diagrams from text descriptions + .claude/skills/plantuml/SKILL.md + + +``` + +## Loading a Skill + +When a user's request matches a skill's purpose, load it using the CLI: + +```bash +skilz read --agent +``` + +**Examples:** +- Claude: `skilz read plantuml` (no --agent needed) +- Gemini: `skilz read plantuml --agent gemini` +- Codex: `skilz read plantuml --agent codex` + +The command outputs: +1. **Base Directory** - Path for resolving bundled resources +2. **SKILL.md Content** - Full instructions to follow + +## Using Bundled Resources + +Skills may include additional files. The base directory path tells you where to find them: + +``` +# Skill: plantuml +# Base Directory: /Users/name/.claude/skills/plantuml +# SKILL.md Path: /Users/name/.claude/skills/plantuml/SKILL.md + +[SKILL.md content follows...] +``` + +To use bundled files, resolve paths relative to the base directory: +- `references/syntax-guide.md` -> `/Users/name/.claude/skills/plantuml/references/syntax-guide.md` +- `scripts/convert.py` -> `/Users/name/.claude/skills/plantuml/scripts/convert.py` + +## Step-by-Step Process + +1. **Identify** - Check `` for a skill matching the user's request +2. **Load** - Run `skilz read --agent ` +3. **Follow** - Execute the instructions in the loaded SKILL.md +4. **Use Resources** - Access bundled scripts/templates via the base directory path + +## Lazy Loading + +Don't load skills preemptively. Load them only when: +- A user explicitly requests the skill's functionality +- The task clearly matches the skill's description +- You need the skill's specific guidance to complete a request + +## Troubleshooting + +**Skill not found:** +```bash +skilz list --agent # See installed skills +skilz install --agent # Install if missing +``` + +**Wrong agent directory:** +Each agent has its own skills directory. Use `--agent` to specify which one. + +**Broken symlink:** +Run `skilz list` to identify broken skills, then reinstall them. diff --git a/src/skilz/installer.py b/src/skilz/installer.py index 730a038..799d340 100644 --- a/src/skilz/installer.py +++ b/src/skilz/installer.py @@ -267,6 +267,7 @@ def install_local_skill( agent=resolved_agent if agent else None, verbose=verbose, target_files=target_files, + force_extended=force_config, ) for result in sync_results: @@ -567,6 +568,7 @@ def install_skill( agent=resolved_agent if agent else None, verbose=verbose, target_files=target_files, + force_extended=force_config, ) # Report what was updated diff --git a/tests/test_config_sync.py b/tests/test_config_sync.py index cba96d1..aa9645d 100644 --- a/tests/test_config_sync.py +++ b/tests/test_config_sync.py @@ -9,6 +9,8 @@ SECTION_END, SECTION_START, SkillReference, + _build_skills_section, + _generate_usage_template, _parse_existing_skills, detect_project_config_files, format_skill_element, @@ -56,7 +58,22 @@ def claude_agent() -> AgentConfig: @pytest.fixture def gemini_agent() -> AgentConfig: - """Create a Gemini agent config.""" + """Create a Gemini agent config with native support (current production config).""" + return AgentConfig( + name="gemini", + display_name="Gemini CLI", + home_dir=Path.home() / ".gemini" / "skills", + project_dir=Path(".gemini") / "skills", + config_files=("GEMINI.md",), + supports_home=True, + default_mode="copy", + native_skill_support="all", + ) + + +@pytest.fixture +def gemini_agent_no_native() -> AgentConfig: + """Create a Gemini agent config without native support (for testing force_extended).""" return AgentConfig( name="gemini", display_name="Gemini CLI", @@ -399,3 +416,204 @@ def test_output_follows_standard_format( assert "" in content assert "" in content assert "" in content + + +class TestGenerateUsageTemplate: + """Tests for _generate_usage_template function.""" + + def test_claude_native_all_no_agent_flag(self) -> None: + """Claude with native_skill_support='all' should not have --agent flag.""" + result = _generate_usage_template("claude", "all") + assert 'skilz read "' in result + assert "--agent" not in result + + def test_codex_native_all_has_agent_flag(self) -> None: + """Codex with native_skill_support='all' should have --agent codex.""" + result = _generate_usage_template("codex", "all") + assert "--agent codex" in result + + def test_gemini_native_none_has_agent_flag(self) -> None: + """Gemini with native_skill_support='none' should have --agent gemini.""" + result = _generate_usage_template("gemini", "none") + assert "--agent gemini" in result + + def test_gemini_native_none_has_extended_instructions(self) -> None: + """Agents with native_skill_support='none' should have step-by-step process.""" + result = _generate_usage_template("gemini", "none") + assert "Step-by-step process:" in result + assert "1. Identify a skill" in result + + def test_opencode_native_home_has_agent_flag(self) -> None: + """OpenCode with native_skill_support='home' should have --agent opencode.""" + result = _generate_usage_template("opencode", "home") + assert "--agent opencode" in result + + def test_opencode_native_home_no_extended_instructions(self) -> None: + """Agents with native_skill_support='home' should NOT have extended instructions.""" + result = _generate_usage_template("opencode", "home") + assert "Step-by-step process:" not in result + + def test_qwen_native_none_has_extended_instructions(self) -> None: + """QWEN with native_skill_support='none' should have extended instructions.""" + result = _generate_usage_template("qwen", "none") + assert "--agent qwen" in result + assert "Step-by-step process:" in result + + +class TestBuildSkillsSectionWithAgent: + """Tests for _build_skills_section with agent parameters.""" + + def test_build_section_claude_no_agent_flag(self, tmp_path: Path) -> None: + """Building section for Claude should not include --agent flag.""" + skill = SkillReference( + skill_id="test/skill", + skill_name="test-skill", + skill_path=tmp_path / "test-skill", + description="Test skill", + ) + (tmp_path / "test-skill").mkdir() + (tmp_path / "test-skill" / "SKILL.md").write_text("# Test") + + result = _build_skills_section( + [skill], + project_dir=tmp_path, + agent_name="claude", + native_support="all", + ) + assert 'skilz read "' in result + assert "--agent" not in result + + def test_build_section_gemini_has_agent_flag(self, tmp_path: Path) -> None: + """Building section for Gemini should include --agent gemini.""" + skill = SkillReference( + skill_id="test/skill", + skill_name="test-skill", + skill_path=tmp_path / "test-skill", + description="Test skill", + ) + (tmp_path / "test-skill").mkdir() + (tmp_path / "test-skill" / "SKILL.md").write_text("# Test") + + result = _build_skills_section( + [skill], + project_dir=tmp_path, + agent_name="gemini", + native_support="none", + ) + assert "--agent gemini" in result + assert "Step-by-step process:" in result + + +class TestUpdateConfigFileAgentSpecific: + """Tests for agent-specific behavior in update_config_file.""" + + def test_claude_config_no_agent_flag( + self, + project_dir: Path, + skill_ref: SkillReference, + claude_agent: AgentConfig, + ) -> None: + """Test that Claude config file doesn't include --agent flag.""" + config_path = project_dir / "CLAUDE.md" + + update_config_file( + config_path=config_path, + skill=skill_ref, + agent_config=claude_agent, + project_dir=project_dir, + create_if_missing=True, + ) + + content = config_path.read_text() + assert 'skilz read "' in content + assert "--agent" not in content + + def test_gemini_native_config_no_extended_instructions( + self, + project_dir: Path, + skill_ref: SkillReference, + gemini_agent: AgentConfig, + ) -> None: + """Test that Gemini with native support doesn't get extended instructions.""" + config_path = project_dir / "GEMINI.md" + + update_config_file( + config_path=config_path, + skill=skill_ref, + agent_config=gemini_agent, + project_dir=project_dir, + create_if_missing=True, + ) + + content = config_path.read_text() + assert "--agent gemini" in content + # Native support means NO extended instructions + assert "Step-by-step process:" not in content + + def test_gemini_no_native_config_has_extended_instructions( + self, + project_dir: Path, + skill_ref: SkillReference, + gemini_agent_no_native: AgentConfig, + ) -> None: + """Test that Gemini without native support gets extended instructions.""" + config_path = project_dir / "GEMINI.md" + + update_config_file( + config_path=config_path, + skill=skill_ref, + agent_config=gemini_agent_no_native, + project_dir=project_dir, + create_if_missing=True, + ) + + content = config_path.read_text() + assert "--agent gemini" in content + assert "Step-by-step process:" in content + + +class TestForceExtendedInstructions: + """Tests for force_extended parameter (--force-config behavior).""" + + def test_force_extended_adds_instructions_for_native_agent( + self, + project_dir: Path, + skill_ref: SkillReference, + gemini_agent: AgentConfig, + ) -> None: + """Test that force_extended adds step-by-step instructions even for native agents.""" + config_path = project_dir / "GEMINI.md" + + # Gemini has native_skill_support="all", but force_extended should add instructions + update_config_file( + config_path=config_path, + skill=skill_ref, + agent_config=gemini_agent, + project_dir=project_dir, + create_if_missing=True, + force_extended=True, + ) + + content = config_path.read_text() + assert "--agent gemini" in content + # force_extended=True should add extended instructions + assert "Step-by-step process:" in content + + def test_generate_usage_template_force_extended(self) -> None: + """Test _generate_usage_template with force_extended=True.""" + # Gemini with native support normally wouldn't get extended instructions + result = _generate_usage_template("gemini", "all", force_extended=False) + assert "Step-by-step process:" not in result + + # But with force_extended=True, it should + result = _generate_usage_template("gemini", "all", force_extended=True) + assert "Step-by-step process:" in result + assert "--agent gemini" in result + + def test_claude_force_extended_still_no_agent_flag(self) -> None: + """Test that Claude still doesn't get --agent flag even with force_extended.""" + result = _generate_usage_template("claude", "all", force_extended=True) + # Claude should never have --agent flag + assert "--agent" not in result + # But should have extended instructions when forced + assert "Step-by-step process:" in result diff --git a/tests/test_read_cmd.py b/tests/test_read_cmd.py index 25e6150..63709e5 100644 --- a/tests/test_read_cmd.py +++ b/tests/test_read_cmd.py @@ -208,7 +208,7 @@ def test_read_with_agent_filter(self, mock_skill: InstalledSkill) -> None: ) def test_read_project_level_fallback(self, mock_skill: InstalledSkill) -> None: - """Test that user-level miss falls back to project-level.""" + """Test that user-level miss falls back to project-level across agents.""" args = argparse.Namespace( skill_name="test-skill", agent=None, @@ -216,14 +216,20 @@ def test_read_project_level_fallback(self, mock_skill: InstalledSkill) -> None: ) with patch("skilz.commands.read_cmd.find_installed_skill") as mock_find: - # First call (user-level) returns None, second (project-level) returns skill - mock_find.side_effect = [None, mock_skill] + # With fallback search, we try all agents at user-level first (5 agents), + # then all agents at project-level. Return skill on first project-level call. + from skilz.commands.read_cmd import FALLBACK_SEARCH_ORDER + + num_agents = len(FALLBACK_SEARCH_ORDER) + # All user-level calls return None, first project-level call returns skill + mock_find.side_effect = [None] * num_agents + [mock_skill] result = cmd_read(args) assert result == 0 - assert mock_find.call_count == 2 - # Second call should have project_level=True + # Should have tried all user-level agents + 1 project-level agent + assert mock_find.call_count == num_agents + 1 + # Last call should have project_level=True _, kwargs = mock_find.call_args assert kwargs.get("project_level") is True @@ -280,3 +286,64 @@ def test_read_command_options(self) -> None: assert args.skill_name == "my-skill" assert args.agent == "claude" assert args.project is True + + +class TestFallbackSearch: + """Tests for fallback directory search in read command.""" + + def test_fallback_search_order_defined(self) -> None: + """Test that FALLBACK_SEARCH_ORDER is defined and has expected agents.""" + from skilz.commands.read_cmd import FALLBACK_SEARCH_ORDER + + assert len(FALLBACK_SEARCH_ORDER) > 0 + assert "claude" in FALLBACK_SEARCH_ORDER + assert "universal" in FALLBACK_SEARCH_ORDER + + def test_fallback_prefers_claude_over_universal(self) -> None: + """Claude directory should be searched before universal directory.""" + from skilz.commands.read_cmd import FALLBACK_SEARCH_ORDER + + claude_idx = FALLBACK_SEARCH_ORDER.index("claude") + universal_idx = FALLBACK_SEARCH_ORDER.index("universal") + assert claude_idx < universal_idx + + def test_fallback_search_finds_skill_in_later_agent(self, mock_skill: InstalledSkill) -> None: + """Test that fallback search finds skill in a later agent directory.""" + args = argparse.Namespace( + skill_name="test-skill", + agent=None, + project=False, + ) + + with patch("skilz.commands.read_cmd.find_installed_skill") as mock_find: + from skilz.commands.read_cmd import FALLBACK_SEARCH_ORDER + + # Skill not found in first few agents, found in 3rd agent + mock_find.side_effect = [None, None, mock_skill] + + result = cmd_read(args) + + assert result == 0 + # Should have called find_installed_skill 3 times + assert mock_find.call_count == 3 + # Third call should be for the 3rd agent in fallback order + third_call_args = mock_find.call_args_list[2] + assert third_call_args[1]["agent"] == FALLBACK_SEARCH_ORDER[2] + + def test_agent_specified_skips_fallback(self, mock_skill: InstalledSkill) -> None: + """Test that specifying --agent skips fallback search.""" + args = argparse.Namespace( + skill_name="test-skill", + agent="gemini", + project=False, + ) + + with patch("skilz.commands.read_cmd.find_installed_skill") as mock_find: + mock_find.return_value = mock_skill + + result = cmd_read(args) + + assert result == 0 + # Should only call once with the specified agent + assert mock_find.call_count == 1 + assert mock_find.call_args[1]["agent"] == "gemini"