diff --git a/README.md b/README.md index 1de743a..0f5e4e4 100644 --- a/README.md +++ b/README.md @@ -60,26 +60,29 @@ For detailed usage instructions, see the [User Manual](docs/USER_MANUAL.md). ## Quick Start ```bash -# Install a skill from the marketplace (defaults to Claude Code, user-level) +# Install a skill from the marketplace using NEW format (defaults to Claude Code, user-level) +skilz install anthropics/skills/algorithmic-art + +# Install using LEGACY format (backwards compatible) skilz install anthropics_skills/algorithmic-art # Install directly from GitHub URL (NEW in 1.5 - no -g flag needed) skilz install https://github.com/owner/repo -# Install for a specific agent -skilz install anthropics_skills/brand-guidelines --agent opencode +# Install for a specific agent using NEW format +skilz install anthropics/skills/brand-guidelines --agent opencode # Install at project level (for sandboxed agents) -skilz install anthropics_skills/frontend-design --agent copilot -p +skilz install anthropics/skills/frontend-design --agent copilot -p # Gemini CLI native support (NEW in 1.7 - requires experimental.skills plugin) -skilz install anthropics_skills/pdf-reader --agent gemini --project +skilz install anthropics/skills/pdf-reader --agent gemini --project # Legacy Gemini workflow (NEW in 1.7 - for users without experimental.skills) -skilz install anthropics_skills/pdf-reader --agent universal --project --config GEMINI.md +skilz install anthropics/skills/pdf-reader --agent universal --project --config GEMINI.md # Universal agent with custom config (NEW in 1.7) -skilz install anthropics_skills/excel --agent universal --project --config CUSTOM.md +skilz install anthropics/skills/excel --agent universal --project --config CUSTOM.md # List installed skills (or use alias: skilz ls) skilz list @@ -98,9 +101,18 @@ skilz read algorithmic-art skilz update # Uninstall a skill (or use alias: skilz rm) -skilz uninstall anthropics_skills/algorithmic-art +skilz uninstall anthropics/skills/algorithmic-art ``` +### Skill ID Formats + +Skilz supports two skill ID formats: + +- **NEW Format (Recommended)**: `owner/repo/skill-name` — e.g., `anthropics/skills/theme-factory` +- **LEGACY Format**: `owner_repo/skill-name` — e.g., `anthropics_skills/theme-factory` + +Both formats work identically. Use whichever you prefer! + Each install command: 1. Resolves the skill from the registry @@ -125,7 +137,12 @@ The skill page for [Theme Factory](https://skillzwave.ai/skill/anthropics_skills skilz install anthropics_skills/theme-factory ``` -The string `anthropics_skills/theme-factory` is the **Skill ID** — the format is `owner_repo/skill-name` where underscores separate owner and repo. +The string `anthropics_skills/theme-factory` is the **Skill ID**. Skilz supports two formats: + +- **NEW Format (Recommended)**: `owner/repo/skill-name` — e.g., `anthropics/skills/theme-factory` +- **LEGACY Format**: `owner_repo/skill-name` — e.g., `anthropics_skills/theme-factory` (underscores separate owner and repo) + +Both formats work identically and resolve to the same skill. The NEW format is more intuitive and matches GitHub repository structure. --- diff --git a/scripts/test_api_integration.sh b/scripts/test_api_integration.sh new file mode 100755 index 0000000..a2b706e --- /dev/null +++ b/scripts/test_api_integration.sh @@ -0,0 +1,217 @@ +#!/usr/bin/env bash +# +# API Integration Test for Skilz CLI +# +# This script specifically tests that the owner_repo/skill format +# correctly calls the REST endpoint at skillzwave.ai/api +# +# Usage: ./scripts/test_api_integration.sh + +set -e + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +# Test skill in marketplace format +SKILL_ID="Jamie-BitFlight_claude_skills/brainstorming-skill" +SKILL_NAME="brainstorming-skill" + +log_info() { + echo -e "${BLUE}[INFO]${NC} $1" +} + +log_success() { + echo -e "${GREEN}[PASS]${NC} $1" +} + +log_fail() { + echo -e "${RED}[FAIL]${NC} $1" +} + +log_section() { + echo "" + echo -e "${BLUE}========================================${NC}" + echo -e "${BLUE} $1${NC}" + echo -e "${BLUE}========================================${NC}" +} + +test_api_endpoint_calls() { + log_section "TEST: API Endpoint Integration" + + # Test 1: Verify API endpoint is reachable + log_info "Testing API endpoint reachability..." + local api_url="https://skillzwave.ai/api/skills/byname" + local test_params="?repoFullName=Jamie-BitFlight/claude_skills&name=brainstorming-skill" + + if curl -s -f "$api_url$test_params" >/dev/null 2>&1; then + log_success "API endpoint is reachable" + else + log_fail "API endpoint is not reachable (may be expected in CI)" + fi + + # Test 2: Install with verbose to see API calls + log_info "Installing with verbose to verify API calls..." + + # Clean up any existing installation + skilz rm "$SKILL_NAME" --agent claude -y 2>/dev/null || true + + # Install with verbose output and capture logs + local install_output + install_output=$(skilz install "$SKILL_ID" --agent claude --verbose 2>&1) + local install_exit_code=$? + + if [[ $install_exit_code -eq 0 ]]; then + log_success "Marketplace install succeeded" + + # Check if API calls are mentioned in verbose output + if echo "$install_output" | grep -q "API lookup\|skillzwave.ai\|/api/"; then + log_success "API calls detected in verbose output" + else + log_info "API calls not explicitly shown (may use local registry)" + fi + + # Verify the skill was installed correctly + if [[ -f "$HOME/.claude/skills/$SKILL_NAME/SKILL.md" ]]; then + log_success "Skill installed correctly via API resolution" + else + log_fail "Skill not found after installation" + fi + + # Check manifest for source information + if [[ -f "$HOME/.claude/skills/$SKILL_NAME/.skilz-manifest.yaml" ]]; then + local manifest_content + manifest_content=$(cat "$HOME/.claude/skills/$SKILL_NAME/.skilz-manifest.yaml") + + if echo "$manifest_content" | grep -q "Jamie-BitFlight/claude_skills"; then + log_success "Manifest shows correct repository resolution" + else + log_fail "Manifest does not show expected repository" + fi + + if echo "$manifest_content" | grep -q "git_sha:"; then + log_success "Manifest contains git SHA (API provided metadata)" + else + log_fail "Manifest missing git SHA" + fi + else + log_fail "Manifest file not found" + fi + + # Clean up + skilz rm "$SKILL_NAME" --agent claude -y 2>/dev/null || true + + else + log_fail "Marketplace install failed" + echo "Install output:" + echo "$install_output" + fi +} + +test_api_format_parsing() { + log_section "TEST: API Format Parsing" + + # Test the format parsing directly with Python + log_info "Testing marketplace format parsing..." + + python3 -c " +import sys +sys.path.insert(0, 'src') +from skilz.api_client import parse_skill_id, is_marketplace_skill_id + +# Test marketplace format detection +skill_id = '$SKILL_ID' +print(f'Testing skill ID: {skill_id}') + +if is_marketplace_skill_id(skill_id): + print('✓ Correctly identified as marketplace format') + + owner, repo, skill = parse_skill_id(skill_id) + print(f'✓ Parsed: owner={owner}, repo={repo}, skill={skill}') + + expected_owner = 'Jamie-BitFlight' + expected_repo = 'claude_skills' + expected_skill = 'brainstorming-skill' + + if owner == expected_owner and repo == expected_repo and skill == expected_skill: + print('✓ Parsing results are correct') + else: + print(f'✗ Parsing mismatch: expected {expected_owner}/{expected_repo}/{expected_skill}') + sys.exit(1) +else: + print('✗ Failed to identify marketplace format') + sys.exit(1) +" + + if [[ $? -eq 0 ]]; then + log_success "Marketplace format parsing works correctly" + else + log_fail "Marketplace format parsing failed" + fi +} + +test_registry_api_fallback() { + log_section "TEST: Registry → API Fallback Logic" + + log_info "Testing registry fallback to API..." + + # Create a temporary directory without any registry + local temp_dir=$(mktemp -d) + cd "$temp_dir" + + # Try to install - should fall back to API since no local registry + log_info "Installing from directory with no local registry..." + + local output + output=$(skilz install "$SKILL_ID" --agent claude --verbose 2>&1) + local exit_code=$? + + if [[ $exit_code -eq 0 ]]; then + log_success "Install succeeded with API fallback" + + if echo "$output" | grep -q "not in local registries\|trying.*API\|marketplace API"; then + log_success "API fallback logic triggered correctly" + else + log_info "API fallback not explicitly shown in output" + fi + + # Clean up + skilz rm "$SKILL_NAME" --agent claude -y 2>/dev/null || true + else + log_fail "Install failed during API fallback test" + echo "Output: $output" + fi + + cd - >/dev/null + rm -rf "$temp_dir" +} + +main() { + echo "" + echo "==============================================" + echo " Skilz API Integration Test" + echo "==============================================" + echo "" + + # Verify skilz is installed + if ! command -v skilz &> /dev/null; then + log_fail "skilz command not found - please install first" + exit 1 + fi + + log_info "Testing with skill: $SKILL_ID" + log_info "Skilz version: $(skilz --version)" + + # Run tests + test_api_format_parsing + test_api_endpoint_calls + test_registry_api_fallback + + echo "" + log_success "API integration tests completed" +} + +main "$@" \ No newline at end of file diff --git a/scripts/test_rest_marketplace_e2e.sh b/scripts/test_rest_marketplace_e2e.sh new file mode 100755 index 0000000..502f216 --- /dev/null +++ b/scripts/test_rest_marketplace_e2e.sh @@ -0,0 +1,433 @@ +#!/usr/bin/env bash +# +# Comprehensive E2E Test Plan for REST Marketplace Endpoint +# +# Tests all three formats (NEW, LEGACY, SLUG) against the live REST API +# at https://skillzwave.ai/api to verify proper behavior +# +# Test Coverage: +# ✅ NEW format resolves correctly +# ✅ LEGACY format resolves correctly +# ✅ SLUG format resolves correctly +# ✅ 404 returned for non-existent skills +# ✅ 400 returned for invalid formats +# ✅ Verbose logging shows REST vs GitHub resolution +# ✅ API endpoint reachability and response validation + +set -e + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' # No Color + +# Test counters +TESTS_PASSED=0 +TESTS_FAILED=0 + +# Test skills - using known skills from the marketplace +VALID_SKILL_OWNER="Jamie-BitFlight" +VALID_SKILL_REPO="claude_skills" +VALID_SKILL_NAME="brainstorming-skill" + +# Format variations of the same skill +NEW_FORMAT="$VALID_SKILL_OWNER/$VALID_SKILL_REPO/$VALID_SKILL_NAME" +LEGACY_FORMAT="${VALID_SKILL_OWNER}_${VALID_SKILL_REPO}/$VALID_SKILL_NAME" +SLUG_FORMAT="${VALID_SKILL_OWNER}__${VALID_SKILL_REPO}__${VALID_SKILL_NAME}" + +# API endpoints +API_BASE="https://skillzwave.ai/api" +API_BYNAME="$API_BASE/skills/byname" +API_COORDINATES="$API_BASE/skills/coordinates" + +log_info() { + echo -e "${BLUE}[INFO]${NC} $1" +} + +log_success() { + echo -e "${GREEN}[PASS]${NC} $1" + ((TESTS_PASSED++)) || true +} + +log_fail() { + echo -e "${RED}[FAIL]${NC} $1" + ((TESTS_FAILED++)) || true +} + +log_section() { + echo "" + echo -e "${BLUE}========================================${NC}" + echo -e "${BLUE} $1${NC}" + echo -e "${BLUE}========================================${NC}" +} + +# Helper function to test API endpoint directly +test_api_endpoint() { + local url="$1" + local expected_status="$2" + local description="$3" + + log_info "Testing: $description" + log_info "URL: $url" + + local response + local status_code + + # Make request and capture both status and response + local temp_file=$(mktemp) + local status_code + status_code=$(curl -s -w "%{http_code}" -o "$temp_file" "$url" 2>/dev/null || echo "000") + local response_body + response_body=$(cat "$temp_file" 2>/dev/null || echo "") + rm -f "$temp_file" + + if [[ "$status_code" == "$expected_status" ]]; then + log_success "API returned expected status $expected_status" + + # If 200, validate JSON structure + if [[ "$status_code" == "200" ]]; then + if echo "$response_body" | jq . >/dev/null 2>&1; then + log_success "Response is valid JSON" + + # Check for required fields + if echo "$response_body" | jq -e '.name, .repoFullName, .skillPath' >/dev/null 2>&1; then + log_success "Response contains required fields (name, repoFullName, skillPath)" + else + log_fail "Response missing required fields" + fi + else + log_fail "Response is not valid JSON" + fi + fi + else + log_fail "API returned status $status_code, expected $expected_status" + if [[ -n "$response_body" ]]; then + echo "Response body: $response_body" + fi + fi + + return 0 +} + +# Helper function to test skilz install with verbose output +test_skilz_install() { + local skill_id="$1" + local format_type="$2" + local expected_result="$3" # "success" or "fail" + local description="$4" + + log_info "Testing skilz install: $description" + log_info "Skill ID: $skill_id" + log_info "Expected format: $format_type" + + # Clean up any existing installation + skilz rm "$(basename "$skill_id")" --agent claude -y 2>/dev/null || true + + # Capture verbose output + local output + local exit_code + + output=$(skilz -v install "$skill_id" --agent claude 2>&1) + exit_code=$? + + # Check format detection + if echo "$output" | grep -q "\[INFO\] Skill ID format: $format_type"; then + log_success "Format correctly detected as $format_type" + else + log_fail "Format not detected as $format_type" + echo "Output: $output" + fi + + # Check REST API attempt + if [[ "$format_type" != "UNKNOWN" ]]; then + if echo "$output" | grep -q "\[INFO\] Attempting REST API lookup"; then + log_success "REST API lookup attempted" + else + log_fail "REST API lookup not attempted" + fi + fi + + # Check result + if [[ "$expected_result" == "success" ]]; then + if [[ $exit_code -eq 0 ]]; then + log_success "Install succeeded as expected" + + # Check for success message + if echo "$output" | grep -q "\[SUCCESS\] REST API resolved skill"; then + log_success "REST API resolution success logged" + else + log_fail "REST API success not logged" + fi + + # Verify skill was actually installed + if [[ -f "$HOME/.claude/skills/$(basename "$skill_id")/SKILL.md" ]]; then + log_success "Skill file actually installed" + else + log_fail "Skill file not found after install" + fi + + else + log_fail "Install failed unexpectedly (exit code: $exit_code)" + echo "Output: $output" + fi + else + if [[ $exit_code -ne 0 ]]; then + log_success "Install failed as expected" + else + log_fail "Install succeeded when it should have failed" + fi + fi + + # Clean up + skilz rm "$(basename "$skill_id")" --agent claude -y 2>/dev/null || true +} + +# Test 1: API Endpoint Reachability +test_api_reachability() { + log_section "TEST 1: API Endpoint Reachability" + + # Test byname endpoint with valid skill + local byname_url="$API_BYNAME?repoFullName=$VALID_SKILL_OWNER/$VALID_SKILL_REPO&name=$VALID_SKILL_NAME" + test_api_endpoint "$byname_url" "200" "Valid skill via byname endpoint" + + # Test coordinates endpoint with valid path + local coords_url="$API_COORDINATES?owner=$VALID_SKILL_OWNER&repo=$VALID_SKILL_REPO&path=$VALID_SKILL_NAME/SKILL.md" + test_api_endpoint "$coords_url" "200" "Valid skill via coordinates endpoint" + + # Test 404 for non-existent skill + local notfound_url="$API_BYNAME?repoFullName=fake-owner/fake-repo&name=fake-skill" + test_api_endpoint "$notfound_url" "404" "Non-existent skill returns 404" +} + +# Test 2: NEW Format Resolution +test_new_format() { + log_section "TEST 2: NEW Format Resolution (owner/repo/skill)" + + test_skilz_install "$NEW_FORMAT" "NEW" "success" "NEW format with valid skill" + + # Test with non-existent skill in NEW format + test_skilz_install "fake-owner/fake-repo/fake-skill" "NEW" "fail" "NEW format with non-existent skill" +} + +# Test 3: LEGACY Format Resolution +test_legacy_format() { + log_section "TEST 3: LEGACY Format Resolution (owner_repo/skill)" + + test_skilz_install "$LEGACY_FORMAT" "LEGACY" "success" "LEGACY format with valid skill" + + # Test with non-existent skill in LEGACY format + test_skilz_install "fake-owner_fake-repo/fake-skill" "LEGACY" "fail" "LEGACY format with non-existent skill" +} + +# Test 4: SLUG Format Resolution +test_slug_format() { + log_section "TEST 4: SLUG Format Resolution (owner__repo__skill)" + + test_skilz_install "$SLUG_FORMAT" "SLUG" "success" "SLUG format with valid skill" + + # Test with non-existent skill in SLUG format + test_skilz_install "fake-owner__fake-repo__fake-skill" "SLUG" "fail" "SLUG format with non-existent skill" +} + +# Test 5: Invalid Format Handling +test_invalid_formats() { + log_section "TEST 5: Invalid Format Handling" + + # Test completely invalid format + test_skilz_install "just-a-name" "UNKNOWN" "fail" "Invalid format (no separators)" + + # Test incomplete NEW format + test_skilz_install "owner/skill" "UNKNOWN" "fail" "Incomplete NEW format (missing repo)" + + # Test incomplete LEGACY format + test_skilz_install "owner/skill" "UNKNOWN" "fail" "Incomplete LEGACY format (no underscore)" +} + +# Test 6: Format Detection Accuracy +test_format_detection() { + log_section "TEST 6: Format Detection Accuracy" + + log_info "Testing format detection with Python API..." + + python3 -c " +import sys +sys.path.insert(0, 'src') +from skilz.api_client import get_skill_id_format, is_marketplace_skill_id, parse_skill_id + +test_cases = [ + ('$NEW_FORMAT', 'new', True), + ('$LEGACY_FORMAT', 'legacy', True), + ('$SLUG_FORMAT', 'slug', True), + ('just-a-name', 'unknown', False), + ('owner/skill', 'unknown', False), + ('https://github.com/owner/repo', 'unknown', False), +] + +all_passed = True +for skill_id, expected_format, expected_marketplace in test_cases: + actual_format = get_skill_id_format(skill_id) + actual_marketplace = is_marketplace_skill_id(skill_id) + + if actual_format == expected_format and actual_marketplace == expected_marketplace: + print(f'✅ {skill_id} -> {actual_format} (marketplace: {actual_marketplace})') + else: + print(f'❌ {skill_id} -> Expected: {expected_format}/{expected_marketplace}, Got: {actual_format}/{actual_marketplace}') + all_passed = False + +# Test parsing for valid formats +try: + owner, repo, skill = parse_skill_id('$NEW_FORMAT') + if owner == '$VALID_SKILL_OWNER' and repo == '$VALID_SKILL_REPO' and skill == '$VALID_SKILL_NAME': + print('✅ NEW format parsing correct') + else: + print(f'❌ NEW format parsing failed: {owner}/{repo}/{skill}') + all_passed = False + + owner, repo, skill = parse_skill_id('$LEGACY_FORMAT') + if owner == '$VALID_SKILL_OWNER' and repo == '$VALID_SKILL_REPO' and skill == '$VALID_SKILL_NAME': + print('✅ LEGACY format parsing correct') + else: + print(f'❌ LEGACY format parsing failed: {owner}/{repo}/{skill}') + all_passed = False + + owner, repo, skill = parse_skill_id('$SLUG_FORMAT') + if owner.lower() == '$(echo $VALID_SKILL_OWNER | tr A-Z a-z)' and repo.lower() == '$(echo $VALID_SKILL_REPO | tr A-Z a-z)' and skill.lower() == '$(echo $VALID_SKILL_NAME | tr A-Z a-z)': + print('✅ SLUG format parsing correct (normalized to lowercase)') + else: + print(f'❌ SLUG format parsing failed: {owner}/{repo}/{skill}') + all_passed = False + +except Exception as e: + print(f'❌ Parsing error: {e}') + all_passed = False + +sys.exit(0 if all_passed else 1) +" + + if [[ $? -eq 0 ]]; then + log_success "All format detection tests passed" + else + log_fail "Format detection tests failed" + fi +} + +# Test 7: REST vs GitHub Fallback Logging +test_rest_vs_github_logging() { + log_section "TEST 7: REST vs GitHub Fallback Logging" + + log_info "Testing that REST API is attempted first for marketplace formats..." + + # Test with a skill that should be found via REST API + local output + output=$(skilz -v install "$NEW_FORMAT" --agent claude 2>&1) + + # Check that REST API is attempted before any GitHub operations + if echo "$output" | grep -q "\[INFO\] Attempting REST API lookup" && \ + echo "$output" | grep -q "API lookup by name:"; then + log_success "REST API attempted first as expected" + else + log_fail "REST API not attempted first" + echo "Output: $output" + fi + + # Clean up + skilz rm "$VALID_SKILL_NAME" --agent claude -y 2>/dev/null || true +} + +# Test 8: Error Response Validation +test_error_responses() { + log_section "TEST 8: Error Response Validation" + + log_info "Testing API error responses..." + + # Test 404 response structure + local notfound_url="$API_BYNAME?repoFullName=definitely-fake-owner/definitely-fake-repo&name=definitely-fake-skill" + local response + response=$(curl -s "$notfound_url" 2>/dev/null || echo "") + + if [[ -n "$response" ]]; then + log_success "404 endpoint returned response" + + # Check if it's valid JSON error response + if echo "$response" | jq -e '.error' >/dev/null 2>&1; then + log_success "404 response contains error field" + else + log_info "404 response format: $response" + fi + else + log_fail "404 endpoint returned no response" + fi +} + +# Main execution +main() { + echo "" + echo "==============================================" + echo " REST Marketplace Endpoint E2E Test Suite" + echo "==============================================" + echo "" + echo "Testing against: $API_BASE" + echo "Valid test skill: $NEW_FORMAT" + echo "" + + # Verify skilz is installed + if ! command -v skilz &> /dev/null; then + log_fail "skilz command not found - please install first" + exit 1 + fi + + log_info "Skilz version: $(skilz --version)" + + # Verify jq is available for JSON parsing + if ! command -v jq &> /dev/null; then + log_fail "jq command not found - please install jq for JSON parsing" + exit 1 + fi + + # Run all tests + test_api_reachability + test_format_detection + test_new_format + test_legacy_format + test_slug_format + test_invalid_formats + test_rest_vs_github_logging + test_error_responses + + # Summary + echo "" + echo "==============================================" + echo " TEST SUMMARY" + echo "==============================================" + echo "" + + local total=$((TESTS_PASSED + TESTS_FAILED)) + echo "Tests Passed: $TESTS_PASSED" + echo "Tests Failed: $TESTS_FAILED" + echo "Total Tests: $total" + echo "" + + if [[ $TESTS_FAILED -eq 0 ]]; then + echo -e "${GREEN}✅ ALL TESTS PASSED!${NC}" + echo "" + echo "✅ NEW format resolves correctly" + echo "✅ LEGACY format resolves correctly" + echo "✅ SLUG format resolves correctly" + echo "✅ 404 returned for non-existent skills" + echo "✅ Invalid formats handled correctly" + echo "✅ REST API attempted first for marketplace formats" + echo "✅ Verbose logging shows resolution method" + echo "" + exit 0 + else + echo -e "${RED}❌ $TESTS_FAILED TESTS FAILED${NC}" + echo "" + echo "Please review the failed tests above and fix any issues." + exit 1 + fi +} + +# Run main with error handling +main "$@" \ No newline at end of file diff --git a/src/skilz/api_client.py b/src/skilz/api_client.py index f961b8d..8d15105 100644 --- a/src/skilz/api_client.py +++ b/src/skilz/api_client.py @@ -33,10 +33,13 @@ def parse_skill_id(skill_id: str) -> tuple[str, str, str]: """ Parse a skill ID into owner, repo, and skill_name. - Format: {owner}_{repo}/{skill_name} + Supports three formats: + - NEW: owner/repo/skill_name (2 slashes) + - LEGACY: owner_repo/skill_name (1 slash, underscore separates owner/repo) + - SLUG: owner__repo__skill (double underscores, advanced users) Args: - skill_id: The skill ID to parse (e.g., "Jamie-BitFlight_claude_skills/clang-format") + skill_id: The skill ID to parse Returns: Tuple of (owner, repo, skill_name) @@ -44,48 +47,98 @@ def parse_skill_id(skill_id: str) -> tuple[str, str, str]: Raises: ValueError: If the skill_id format is invalid """ - if "/" not in skill_id: + # Check for SLUG format (double underscores) + if "__" in skill_id: + parts = skill_id.split("__") + if len(parts) >= 3: + owner = parts[0] + repo = parts[1] + skill_name = "__".join(parts[2:]) # Handle skill names with __ + if owner and repo and skill_name: + return owner.lower(), repo.lower(), skill_name.lower() + raise ValueError(f"Invalid slug format: '{skill_id}'. Expected format: owner__repo__skill") + + # Count slashes to determine format + slash_count = skill_id.count("/") + + # NEW format: owner/repo/skill (2 slashes) + if slash_count == 2: + owner, repo, skill_name = skill_id.split("/") + if owner and repo and skill_name: + return owner, repo, skill_name raise ValueError( - f"Invalid skill ID format: '{skill_id}'. Expected format: owner_repo/skill_name" + f"Invalid skill ID format: '{skill_id}'. Expected format: owner/repo/skill_name" ) - owner_repo, skill_name = skill_id.rsplit("/", 1) - - if "_" not in owner_repo: - raise ValueError( - f"Invalid skill ID format: '{skill_id}'. " - "Expected format: owner_repo/skill_name (with underscore between owner and repo)" - ) - - # Split on FIRST underscore (GitHub owners can't contain underscores, only hyphens) - idx = owner_repo.find("_") - owner = owner_repo[:idx] - repo = owner_repo[idx + 1 :] - - if not owner or not repo or not skill_name: + # LEGACY format: owner_repo/skill (1 slash with underscore) + if slash_count == 1: + owner_repo, skill_name = skill_id.rsplit("/", 1) + if "_" not in owner_repo: + raise ValueError( + f"Invalid skill ID format: '{skill_id}'. " + "Expected format: owner_repo/skill_name or owner/repo/skill_name" + ) + # Split on FIRST underscore (GitHub owners can have hyphens, not underscores) + idx = owner_repo.find("_") + owner = owner_repo[:idx] + repo = owner_repo[idx + 1 :] + if owner and repo and skill_name: + return owner, repo, skill_name raise ValueError( f"Invalid skill ID format: '{skill_id}'. " "Owner, repo, and skill_name must all be non-empty" ) - return owner, repo, skill_name + raise ValueError( + f"Invalid skill ID format: '{skill_id}'. " + "Expected: owner/repo/skill, owner_repo/skill, or owner__repo__skill" + ) def is_marketplace_skill_id(skill_id: str) -> bool: """ - Check if a skill ID is in the marketplace format. + Check if a skill ID is in a recognized marketplace format. - Marketplace format: {owner}_{repo}/{skill_name} - Legacy format: {owner}/{skill_name} (no underscore before /) + Recognized formats: + - NEW: owner/repo/skill_name (2 slashes) + - LEGACY: owner_repo/skill_name (1 slash with underscore) + - SLUG: owner__repo__skill (double underscores) Returns: True if the skill ID appears to be a marketplace skill ID """ - if "/" not in skill_id: - return False + # SLUG format + if "__" in skill_id: + return skill_id.count("__") >= 2 - owner_repo, _ = skill_id.rsplit("/", 1) - return "_" in owner_repo + # NEW format: 2 slashes + if skill_id.count("/") == 2: + return True + + # LEGACY format: 1 slash with underscore before it + if "/" in skill_id: + owner_repo, _ = skill_id.rsplit("/", 1) + return "_" in owner_repo + + return False + + +def get_skill_id_format(skill_id: str) -> str: + """ + Determine the format type of a skill ID. + + Returns: + One of: 'new', 'legacy', 'slug', 'unknown' + """ + if "__" in skill_id and skill_id.count("__") >= 2: + return "slug" + if skill_id.count("/") == 2: + return "new" + if "/" in skill_id: + owner_repo, _ = skill_id.rsplit("/", 1) + if "_" in owner_repo: + return "legacy" + return "unknown" def fetch_skill_by_name( diff --git a/src/skilz/installer.py b/src/skilz/installer.py index 7a3f178..80cf02f 100644 --- a/src/skilz/installer.py +++ b/src/skilz/installer.py @@ -14,6 +14,7 @@ get_agent_display_name, supports_home_install, ) +from skilz.api_client import get_skill_id_format from skilz.config_sync import SkillReference, sync_skill_to_configs from skilz.errors import InstallError from skilz.git_ops import ( @@ -345,11 +346,18 @@ def install_skill( # Step 2: Look up skill in registry if verbose: + format_type = get_skill_id_format(skill_id) print(f"Looking up skill: {skill_id}") + print(f" [INFO] Skill ID format: {format_type.upper()}") + if format_type in ["new", "legacy", "slug"]: + print(" [INFO] Attempting REST API lookup at skillzwave.ai...") skill_info: SkillInfo = lookup_skill(skill_id, verbose=verbose) if verbose: + format_type = get_skill_id_format(skill_id) + if format_type in ["new", "legacy", "slug"]: + print(f" [SUCCESS] REST API resolved skill: {skill_id}") print(f" Found: {skill_info.git_repo}") print(f" Path: {skill_info.skill_path}") print(f" SHA: {skill_info.git_sha[:8]}...") diff --git a/tests/test_api_client.py b/tests/test_api_client.py index 28c3ace..85b5203 100644 --- a/tests/test_api_client.py +++ b/tests/test_api_client.py @@ -6,6 +6,7 @@ from skilz.api_client import ( fetch_skill_coordinates, + get_skill_id_format, is_marketplace_skill_id, parse_skill_id, ) @@ -39,7 +40,7 @@ def test_repo_with_underscores(self): def test_invalid_no_slash(self): """Test that missing slash raises ValueError.""" - with pytest.raises(ValueError, match="Expected format"): + with pytest.raises(ValueError, match="Expected:"): parse_skill_id("no-slash-here") def test_invalid_no_underscore(self): @@ -124,3 +125,84 @@ def test_tries_multiple_paths(self, mock_request): assert result.skill_path == "skills/skill/SKILL.md" assert mock_request.call_count == 2 + + +class TestParseSkillIdNewFormat: + """Tests for NEW format: owner/repo/skill""" + + def test_new_format_basic(self): + """Test NEW format: owner/repo/skill""" + owner, repo, skill = parse_skill_id("davila7/claude-code-templates/slack-gif-creator") + assert owner == "davila7" + assert repo == "claude-code-templates" + assert skill == "slack-gif-creator" + + def test_new_format_with_hyphens(self): + """Test NEW format with hyphens in all parts""" + owner, repo, skill = parse_skill_id("my-org/my-repo/my-skill") + assert owner == "my-org" + assert repo == "my-repo" + assert skill == "my-skill" + + +class TestParseSkillIdSlugFormat: + """Tests for SLUG format: owner__repo__skill""" + + def test_slug_format(self): + """Test SLUG format: owner__repo__skill""" + owner, repo, skill = parse_skill_id("davila7__claude-code-templates__slack-gif-creator") + assert owner == "davila7" + assert repo == "claude-code-templates" + assert skill == "slack-gif-creator" + + def test_slug_format_lowercase(self): + """Test SLUG format normalizes to lowercase""" + owner, repo, skill = parse_skill_id("Davila7__Claude-Code-Templates__Slack-GIF-Creator") + assert owner == "davila7" + assert repo == "claude-code-templates" + assert skill == "slack-gif-creator" + + +class TestIsMarketplaceSkillIdFormats: + """Tests for is_marketplace_skill_id with all formats""" + + def test_new_format_is_marketplace(self): + """NEW format is recognized as marketplace""" + assert is_marketplace_skill_id("owner/repo/skill") is True + + def test_legacy_format_is_marketplace(self): + """LEGACY format is recognized as marketplace""" + assert is_marketplace_skill_id("owner_repo/skill") is True + + def test_slug_format_is_marketplace(self): + """SLUG format is recognized as marketplace""" + assert is_marketplace_skill_id("owner__repo__skill") is True + + def test_github_url_not_marketplace(self): + """GitHub URL is not marketplace format""" + assert is_marketplace_skill_id("https://github.com/owner/repo") is False + + def test_simple_path_not_marketplace(self): + """Simple path is not marketplace format""" + assert is_marketplace_skill_id("just-a-name") is False + + +class TestGetSkillIdFormat: + """Tests for get_skill_id_format()""" + + def test_new_format_detected(self): + """NEW format correctly detected""" + assert get_skill_id_format("owner/repo/skill") == "new" + + def test_legacy_format_detected(self): + """LEGACY format correctly detected""" + assert get_skill_id_format("owner_repo/skill") == "legacy" + + def test_slug_format_detected(self): + """SLUG format correctly detected""" + assert get_skill_id_format("owner__repo__skill") == "slug" + + def test_unknown_format_detected(self): + """Unknown format correctly detected""" + assert get_skill_id_format("just-a-name") == "unknown" + assert get_skill_id_format("owner/skill") == "unknown" # Missing repo