-
-
Notifications
You must be signed in to change notification settings - Fork 524
fix: Update setup_mcp.sh for v2.0.0 src/ layout + test fixes #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Phase 1: Fix race condition where main console exits before enhancement completes Changes to enhance_skill_local.py: - Add headless mode (default) using subprocess.run() which WAITS for completion - Add timeout protection (default 10 minutes, configurable) - Verify SKILL.md was actually updated (check mtime and size) - Add --interactive-enhancement flag to use old terminal mode - Detailed progress messages and error handling - Clean up temp files after completion Changes to doc_scraper.py: - Use skill-seekers-enhance entry point instead of direct python path - Pass --interactive-enhancement flag through if requested - Update help text to reflect new headless default behavior - Show proper status messages (HEADLESS vs INTERACTIVE) Benefits: - Main console now waits for enhancement to complete - No more "Package your skill" message while enhancement is running - Timeout prevents infinite hangs - Terminal mode still available for users who want it - Better error messages and progress tracking Fixes user request: "make sure 1. console wait for it to finish" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Phase 2 & 3: Quality assurance before packaging New module: quality_checker.py - Enhancement verification (checks for template text, code examples, sections) - Structure validation (SKILL.md, references/ directory) - Content quality checks (YAML frontmatter, language tags, "When to Use" section) - Link validation (internal markdown links) - Quality scoring system (0-100 score + A-F grade) - Detailed reporting with errors, warnings, and info messages - CLI with --verbose and --strict modes Integration in package_skill.py: - Automatic quality checks before packaging - Display quality report with score and grade - Ask user to confirm if warnings/errors found - Add --skip-quality-check flag to bypass checks - Updated help examples Benefits: - Catch quality issues before packaging - Ensure SKILL.md is properly enhanced - Validate all links work - Give users confidence in skill quality - Comprehensive quality reports Addresses user request: "check some sort of quality check at the end like is links working, skill is good etc and give report the user" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Phase 4: Testing and verification New test file: test_quality_checker.py - 12 comprehensive tests for quality checker functionality - Tests for structure validation (missing SKILL.md, missing references) - Tests for enhancement verification (template indicators, code examples) - Tests for content quality (YAML frontmatter, language tags) - Tests for link validation (broken internal links) - Tests for quality scoring and grading system - Tests for is_excellent property - CLI tests (help output, nonexistent directory) Updated test_package_skill.py: - Added skip_quality_check=True to all test calls - Fixes OSError "reading from stdin while output is captured" - All 9 package_skill tests passing Test Results: - 391 tests passing (up from 386 before) - 32 skipped - 0 failures - Added 12 new quality checker tests - All existing tests still passing Completes Phase 4 of enhancement race condition fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The MCP server's package_skill_tool was failing in CI because the quality checker was prompting for user input, which doesn't exist in CI/MCP contexts. Fix: - Add --skip-quality-check flag to package_skill command in MCP server - This prevents interactive prompts that cause EOFError in CI - MCP tools should skip interactive checks since they run in background Impact: - All 25 MCP server tests now pass - All 391 tests passing - CI builds will succeed Context: - Quality checks are interactive by default for CLI users - MCP server runs commands programmatically without user input - This is the correct behavior: interactive for CLI, non-interactive for MCP 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The terminal detection tests were failing because they expected the old terminal mode behavior, but headless mode is now the default. Fix: - Add headless=False parameter to all terminal detection tests - Tests now explicitly test interactive (terminal) mode - test_subprocess_popen_called_with_correct_args: Tests terminal launch - test_terminal_launch_error_handling: Tests error handling - test_output_message_unknown_terminal: Tests warning messages These tests only run on macOS (they're skipped on Linux) and test the interactive terminal launch functionality, so they need headless=False. Impact: - All 3 failing macOS tests should now pass - 391 tests passing on Linux - CI should pass on macOS now 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Major enhancement release focusing on quality and reliability: ## Key Features ### Comprehensive Quality Checker - Automatic quality validation before packaging - Quality scoring system (0-100 + A-F grades) - Enhancement verification (code examples, sections) - Structure validation (SKILL.md, references/) - Content quality checks (frontmatter, language tags) - Link validation (internal markdown links) - Detailed reporting with errors, warnings, and info ### Headless Enhancement Mode (Default) - Runs enhancement in background (no terminal windows) - Main console waits for completion (no race conditions) - 10-minute timeout protection (configurable) - Verification that SKILL.md was actually updated - Interactive mode still available via --interactive-enhancement ## Statistics - 391 tests passing (up from 379) - +12 quality checker tests - All CI checks passing - 5 commits in this release ## Breaking Changes - Enhancement now runs in headless mode by default - Use --interactive-enhancement for old terminal mode behavior See CHANGELOG.md for full details and migration guide. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The Release workflow was failing with ModuleNotFoundError because it wasn't installing the package before running tests. Added 'pip install -e .' step to install the skill_seekers package in editable mode, which is required for the src/ layout structure introduced in v2.0.0. This is the same fix applied to the Tests workflow earlier. Fixes the failing Release check for v2.1.0 tag.
- Fixed MCP server path: skill_seeker_mcp/server.py → src/skill_seekers/mcp/server.py - Updated installation method: Use pip install -e . instead of separate requirements - Updated all path references throughout the script - Aligned with v2.0.0 modern Python packaging structure This fixes the setup_mcp.sh script to work with the new src/ layout introduced in v2.0.0. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Update 4 failing tests to match new v2.0.0 src/ layout: 1. test_references_correct_mcp_directory - Now expects src/skill_seekers/mcp/ (not skill_seeker_mcp/) 2. test_requirements_txt_path - Now checks for '-e .' (editable install) - No longer expects requirements.txt files 3. test_server_py_path - Now expects src/skill_seekers/mcp/server.py 4. test_json_config_path_format - Now expects "$REPO_PATH/src/skill_seekers/mcp/server.py" These test updates align with PR #197's setup_mcp.sh changes that migrated from old skill_seeker_mcp/ paths to modern src/skill_seekers/mcp/ layout introduced in v2.0.0. All 11 TestSetupMCPScript tests now pass.
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #192 - Updates setup_mcp.sh to work with v2.0.0 src/ layout and updates corresponding tests.
This PR combines:
Changes
setup_mcp.sh (from PR #197)
skill_seeker_mcp/server.py→src/skill_seekers/mcp/server.pypip install -r requirements.txt→pip install -e .(modern packaging)tests/test_setup_scripts.py (added)
Fixed 4 failing tests to match v2.0.0 paths:
test_references_correct_mcp_directory
src/skill_seekers/mcp/(notskill_seeker_mcp/)test_requirements_txt_path
-e .(editable install)test_server_py_path
src/skill_seekers/mcp/server.pytest_json_config_path_format
"$REPO_PATH/src/skill_seekers/mcp/server.py"Testing
Related
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Co-Authored-By: wangmeng5 wangmeng5@ideal.com.cn