From bedb23778054fbaadf27bbea3d037251f600be0c Mon Sep 17 00:00:00 2001 From: Jazzcort Date: Fri, 8 May 2026 12:50:51 -0400 Subject: [PATCH] feat: add Claude Code skills and expand agent documentation Add three Claude Code skills (generate-commit-message, prepare-for-pull-request, review-pr) for workflow automation. Create CLAUDE.md with core rules. Expand AGENTS.md with architecture, project layout, tools overview, configuration reference, and tech stack details. --- .../skills/generate-commit-message/SKILL.md | 82 ++++++++ .../skills/prepare-for-pull-request/SKILL.md | 92 +++++++++ .claude/skills/review-pr/SKILL.md | 182 ++++++++++++++++++ AGENTS.md | 146 +++++++++++++- CLAUDE.md | 13 ++ 5 files changed, 507 insertions(+), 8 deletions(-) create mode 100644 .claude/skills/generate-commit-message/SKILL.md create mode 100644 .claude/skills/prepare-for-pull-request/SKILL.md create mode 100644 .claude/skills/review-pr/SKILL.md create mode 100644 CLAUDE.md diff --git a/.claude/skills/generate-commit-message/SKILL.md b/.claude/skills/generate-commit-message/SKILL.md new file mode 100644 index 00000000..4adc1859 --- /dev/null +++ b/.claude/skills/generate-commit-message/SKILL.md @@ -0,0 +1,82 @@ +--- +name: generate-commit-message +description: Use git diff to analyze changes and generate a conventional commit message following this project's conventions from AGENTS.md +disable-model-invocation: true +allowed-tools: Bash(git diff *) Bash(git status) Bash(git log *) +--- + +Generate a commit message for the current changes by following these steps: + +## 1. Parse arguments + +`$ARGUMENTS` may contain additional context for the commit message. + +## 2. Gather context + +- Run `git status` to see what files are staged vs unstaged. +- Run `git diff --cached` to see staged changes. If nothing is staged, inform the user that nothing is staged and stop — do not fall back to unstaged changes. +- Run `git log --oneline -10` to see recent commit messages for style reference. + +## 3. Analyze the changes + +Identify the **type** based on the nature of the change. This project uses both standard Conventional Commit types and module-level types: + +**Standard types:** `feat`, `fix`, `refactor`, `test`, `docs`, `ci`, `chore`, `perf`, `style`, `build` + +**Module-level types** — used when the change is entirely within one of these subsystems: +- `run_script` — changes scoped to `tools/run_script.py` or the run_script workflow +- `eval` — changes to the gatekeeper evaluation/benchmark system + +Identify the **scope** from the area of the codebase affected. Use these established scopes: + +| Scope | When to use | +|-------|------------| +| `ssh` | `connection/ssh.py` or SSH-related logic | +| `run_script` | `tools/run_script.py` (use as scope with `fix`/`feat`, or as standalone type) | +| `deps` | dependency version bumps (`chore(deps)`) | +| `ci` | CI/CD workflows in `.github/` | +| `scripts` | utility scripts in `scripts/` | +| `functional` | functional/integration tests in `tests/functional/` | +| `storage` | `tools/storage.py` or storage-related tools | +| `models` | `models.py` data model changes | +| `validation` | `utils/validation.py` input validation | +| `mcp-apps(ui)` | React UI in `mcp-app/` (nested scope) | + +**Scope rules:** +- Omit scope when the change spans multiple areas or no established scope fits. +- When using a module-level type like `run_script:` or `eval:`, do not add a redundant scope. +- For dependency updates, always use `chore(deps):`. + +## 4. Generate the commit message + +Follow the [Conventional Commits](https://www.conventionalcommits.org/) format: + +``` +type(scope): short imperative description +``` + +Or for module-level types: + +``` +type: short imperative description +``` + +**Subject line rules:** +- Imperative mood, lowercase start, no period at the end. +- Under 72 characters total. +- Focus on **why** or **what changed**, not how. +- If there is a `$ARGUMENTS` value, use it as additional context for the message. + +**Body** (optional — only when the subject line alone doesn't capture the reasoning): +``` + +Motivation and context behind the change. +Wrap at 72 characters per line. +``` + +## 5. Present the result + +Show the generated commit message in a code block so the user can review it. Ask the user if they'd like to: +1. Use the message as-is (run `git commit` with it) +2. Edit it before committing +3. Regenerate with different emphasis diff --git a/.claude/skills/prepare-for-pull-request/SKILL.md b/.claude/skills/prepare-for-pull-request/SKILL.md new file mode 100644 index 00000000..d2df9260 --- /dev/null +++ b/.claude/skills/prepare-for-pull-request/SKILL.md @@ -0,0 +1,92 @@ +--- +name: prepare-for-pull-request +description: Run make verify, then summarize changes since a given commit SHA into a PR description +disable-model-invocation: true +allowed-tools: Bash(make *) Bash(git diff *) Bash(git log *) Bash(git status) Bash(git rev-parse *) Bash(git reflog *) +argument-hint: "[base-commit-sha]" +--- + +Prepare a pull request description by verifying the code and summarizing all changes since a base commit. + +## Step 0: Determine the base ref + +If `$ARGUMENTS` is provided (e.g. a SHA, `main`, `HEAD~3`), use it as the base ref. + +If `$ARGUMENTS` is empty, auto-detect the branch point: + +1. Get the current branch name: `git rev-parse --abbrev-ref HEAD` +2. Find where this branch was created from using reflog: `git reflog show --format='%H %gs'` +3. Look for the entry with `branch: Created from` in the reflog output — the SHA on that line is where the branch was born. +4. Use that SHA as the base ref. Tell the user which base ref was auto-detected so they can confirm. + +If the reflog doesn't contain a `Created from` entry (e.g. the reflog was pruned), fall back to asking the user for the base ref. + +## Step 1: Run verification + +Run `make verify` to sync dependencies and execute all CI checks (lint, format, types, tests). + +- If it passes, proceed to step 2. +- If it fails, stop and show the user the errors. Offer to help fix them before continuing. Do NOT proceed to PR description generation until verification passes. + +## Step 2: Gather change information + +Run these commands to understand the full scope of changes: + +- `git log --oneline $ARGUMENTS..HEAD` — list all commits being included. +- `git log --format="### %s%n%n%b" $ARGUMENTS..HEAD` — get full commit messages with bodies. +- `git diff --stat $ARGUMENTS..HEAD` — file-level summary of what changed. +- `git diff $ARGUMENTS..HEAD` — full diff for detailed analysis. + +**Uncommitted changes:** By default, only include committed changes in the PR description. Ignore uncommitted (staged or unstaged) files unless the user explicitly mentions or asks to include them. + +## Step 3: Analyze and categorize + +Review all commits and the diff to understand: + +- **The problem or gap that motivated this work** — what was missing, broken, or insufficient before this PR? Derive this from commit messages, code comments, and the nature of the changes themselves. +- What features were added, bugs fixed, or refactors made. +- Which project components were affected — map changes to the project architecture: + - **Tools** (`tools/*.py`) — which MCP tools were added or modified? + - **Parsers/Formatters** (`parsers.py`, `formatters.py`) — output parsing or display changes? + - **Connection** (`connection/ssh.py`) — SSH or remote execution changes? + - **Models** (`models.py`) — data model changes? + - **Config** (`config.py`) — new env vars or configuration options? + - **Gatekeeper** (`gatekeeper/`) — script validation changes? + - **MCP App** (`mcp-app/`) — UI changes? + - **Tests** (`tests/`) — new or modified test coverage? + - **CI/Docs/Scripts** — infrastructure changes? +- Any breaking changes, new dependencies, or new `LINUX_MCP_*` configuration variables. + +## Step 4: Generate the PR description + +Produce a PR description in this format: + +```markdown +## Motivation + + +## Summary + + +## Changes + + +## Test plan + + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +``` + +Rules: +- **"Motivation" comes first** and must answer "what problem does this solve?" — not "what does this PR do?" +- Keep the summary concise — focus on the approach, not a line-by-line rehash. +- In the Changes section, group by project component (Tools, Connection, Models, Config, Tests, etc.) rather than generic categories. +- Call out breaking changes, migration steps, or new `LINUX_MCP_*` env vars prominently. +- Mention new dependencies or changes to `pyproject.toml` if applicable. + +## Step 5: Present the result + +Show the generated PR description in a code block. Ask the user if they'd like to: +1. Create the PR now using `gh pr create` with this description +2. Edit the description first +3. Regenerate with different emphasis diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md new file mode 100644 index 00000000..30e229cd --- /dev/null +++ b/.claude/skills/review-pr/SKILL.md @@ -0,0 +1,182 @@ +--- +name: review-pr +description: Review a pull request by URL for code format, types, semantic correctness, readability, redundant logic, bugs, and security concerns +disable-model-invocation: true +allowed-tools: Bash(gh pr *) Bash(gh api *) Bash(git diff *) Bash(git log *) Bash(git status) Bash(git fetch *) Bash(git checkout *) Bash(git rev-parse *) Bash(git merge-base *) Bash(make lint) Bash(make format) Bash(make types) Bash(make test) Read +argument-hint: "" +--- + +Review a pull request for code quality issues across six dimensions: format, types, semantic correctness, readability, redundant logic, and bugs/security. + +## Step 0: Fetch PR information + +`$ARGUMENTS` must be a GitHub PR URL (e.g. `https://github.com/owner/repo/pull/123`). + +If `$ARGUMENTS` is empty, ask the user for the PR URL. Do not proceed without it. + +1. Extract the PR number from the URL. +2. Run `gh pr view --json number,title,baseRefName,headRefName,commits,files` to get PR metadata. +3. Fetch the PR branch locally: + - `git fetch origin pull//head:pr-` to create a local ref for the PR. + - Determine the merge base: `git merge-base origin/ pr-` +4. Use the merge base as the base ref for all diff commands. + +Tell the user which PR you're reviewing (number, title, head branch, base branch). + +## Step 1: Gather changes + +Run these commands to understand the full scope of changes: + +- `git log --oneline ..pr-` — list all commits in the PR. +- `git diff --stat ..pr-` — file-level summary of what changed. +- `git diff ..pr-` — full diff for detailed analysis. + +Read every changed file in full (from the PR branch) so you can evaluate changes in their surrounding context, not just the diff hunks in isolation. Use `git show pr-:` to read files from the PR branch without switching branches. + +## Step 2: Format check + +Run `make lint` and `make format` against the PR branch to check for formatting and linting violations. + +To do this without modifying the current working tree: +- Check out the PR branch: `git checkout pr-` +- Run `make lint` and `make format`. +- After the review is complete (Step 8), check out the original branch. + +Report results: +- **Pass** — no issues found. +- **Fail** — list every violation with file path, line number, and the rule ID. Group by file. + +Do NOT auto-fix. Only report. + +## Step 3: Type check + +Run `make types` to run pyright on the PR branch. + +Report results: +- **Pass** — no type errors. +- **Fail** — list every error with file path, line number, and the pyright message. Group by file. + +For the `mcp-app/` TypeScript code, if any `.ts` or `.tsx` files are in the diff, note that TypeScript type checking should be run separately inside `mcp-app/` if a tsconfig and type-check script exist there. + +## Step 4: Semantic review + +Analyze every changed file for semantic correctness. For each file, check: + +1. **Behavioral changes** — Does the change alter existing behavior? Is that intentional or an accidental regression? Look for: + - Changed return values, error handling, or control flow. + - Modified default values or function signatures. + - Altered query/filter logic. + +2. **Edge cases** — Are there inputs or states the new code doesn't handle? + - Empty/null/zero-length inputs. + - Boundary values (off-by-one, max int, empty string vs None). + - Concurrent or async race conditions. + +3. **API contract consistency** — Do changes to models, tool signatures, or config stay consistent across: + - `models.py` ↔ `parsers.py` ↔ `formatters.py` + - `commands.py` ↔ `tools/*.py` + - `config.py` env var names ↔ documentation + +4. **Test coverage** — Are new code paths covered by tests? Flag any added logic that lacks a corresponding test case. + +Report each finding with: +- File path and line number(s). +- What the issue is. +- Severity: **critical** (likely bug/regression), **warning** (potential issue), or **info** (suggestion). + +## Step 5: Readability and redundancy review + +Evaluate the changed code for maintainability: + +1. **Readability issues:** + - Overly complex expressions that should be broken up. + - Unclear variable or function names. + - Deeply nested logic (3+ levels) that could be flattened with early returns or guard clauses. + - Magic numbers or strings that should be named constants. + - Functions doing too many things (violating single responsibility). + +2. **Redundant logic:** + - Duplicate code across the diff or between the diff and existing code. + - Conditions that are always true/false. + - Unnecessary intermediate variables or re-assignments. + - Dead code (unreachable branches, unused imports, unused variables). + - Over-engineered abstractions for simple operations. + +Report each finding with file path, line number(s), what the issue is, and a concrete suggestion for improvement. + +## Step 6: Bug and security review + +Scan changes for bugs and security concerns, with special attention to this project's security model (read-only tools, SSH key auth, input validation): + +1. **Bugs:** + - Incorrect exception handling (swallowing errors, catching too broadly). + - Resource leaks (unclosed connections, file handles). + - Async issues (missing `await`, unawaited coroutines). + - Incorrect use of mutable default arguments. + +2. **Security (OWASP + project-specific):** + - **Command injection** — Any user input reaching shell commands without validation. Check against the project's `commands.py` allowlist pattern. + - **Path traversal** — File path parameters that aren't validated against allowlists (see `ALLOWED_LOG_PATHS`, `MAX_FILE_READ_BYTES`). + - **Information disclosure** — Sensitive data in logs, error messages, or tool outputs. Check that `audit.py` redaction covers new fields. + - **SSH security** — Any weakening of host key verification, key-based auth, or connection pooling. + - **Read-only violation** — Any tool missing `readOnlyHint=True` or performing writes without going through the gatekeeper. + - **Input validation** — Missing or insufficient validation on tool parameters (see `utils/validation.py`). + - **Dependency risks** — New dependencies that are unmaintained, have known CVEs, or pull in excessive transitive deps. + +Rate each finding: +- **critical** — Exploitable vulnerability or guaranteed bug. Must fix before merge. +- **warning** — Potential issue depending on context. Should fix or explicitly justify. +- **info** — Defense-in-depth suggestion or minor hardening opportunity. + +## Step 7: Run tests + +Run `make test` on the PR branch to verify all tests pass. + +Report results: +- **Pass** — all tests green, with coverage summary if printed. +- **Fail** — list failing tests with their error messages. Note whether failures are related to the PR changes or pre-existing. + +## Step 8: Restore original branch and present summary + +Check out the original branch that was active before the review began. + +Present the full review in this format: + +``` +## PR Review: # — +**Branch:** <head> → <base> + +### Format & Lint +<results from Step 2> + +### Type Check +<results from Step 3> + +### Tests +<results from Step 7> + +### Semantic Issues +<findings from Step 4, ordered by severity> + +### Readability & Redundancy +<findings from Step 5> + +### Bugs & Security +<findings from Step 6, ordered by severity> + +### Verdict + +**<APPROVE | REQUEST CHANGES | NEEDS DISCUSSION>** + +<1-3 sentence summary of the overall state of the PR. Call out the most important finding if any.> +``` + +Verdict criteria: +- **APPROVE** — No critical or warning findings. All checks pass. +- **REQUEST CHANGES** — Any critical finding, or 3+ warnings. +- **NEEDS DISCUSSION** — Warnings that involve design decisions or tradeoffs the author should weigh in on. + +After presenting the review, ask the user if they'd like to: +1. Fix the reported issues (offer to help with specific fixes). +2. Re-run the review after making changes. +3. Discuss any specific finding in more detail. diff --git a/AGENTS.md b/AGENTS.md index a100430a..d3a125a0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,23 +1,150 @@ # Linux MCP Server -Read-only MCP server for Linux system diagnostics. +Read-only [Model Context Protocol](https://modelcontextprotocol.io/) server for Linux system administration, diagnostics, and troubleshooting. Targets RHEL-based systems. All tools are read-only by default; an optional guarded command execution feature allows validated script execution with LLM-based safety checks and human approval. -## Commands +## Quick Reference ```bash uv sync # Install dependencies uv run pytest # Run tests uv run ruff check src tests # Lint uv run pyright # Type check -make verify # All checks (required before commit) +make verify # All checks (sync + lint + types + tests) +``` + +## Architecture + +### Fixed Toolset (`LINUX_MCP_TOOLSET=fixed`) + +``` +MCP Client (Claude Desktop, etc.) + | MCP Protocol (stdio / http / streamable-http) +FastMCP Server (server.py - tool discovery, transport, middleware) + | +Tool Modules (tools/*.py - read-only tools) + | +Command Registry (commands.py - immutable command definitions with fallbacks) + | +SSH Executor (connection/ssh.py) / Local Executor + | +Target Linux System (local or remote via SSH) +``` + +### Guarded Command Execution (`LINUX_MCP_TOOLSET=run_script`) + +``` +MCP Client (Claude Desktop, etc.) + | MCP Protocol (stdio / http / streamable-http) +FastMCP Server (server.py - tool discovery, transport, middleware) + | +run_script tools (tools/run_script.py) + | +validate_script ──> Gatekeeper (gatekeeper/check_run_script.py) + | | + | LLM Safety Check (via LiteLLM) + | | + | Risk Assessment (approve / reject / needs-confirmation) + | +SSH Executor (connection/ssh.py) / Local Executor + | +Target Linux System (local or remote via SSH) ``` ## Project Layout -- `src/linux_mcp_server/tools/` - MCP tools (logs, network, processes, services, storage, system_info) -- `src/linux_mcp_server/commands.py` - Command definitions -- `src/linux_mcp_server/formatters.py` / `parsers.py` - Output formatting and parsing -- `tests/` - Mirrors src structure +``` +src/linux_mcp_server/ + server.py # FastMCP setup, middleware, transport config + config.py # Pydantic-based config from LINUX_MCP_* env vars + commands.py # Centralized command registry (MappingProxyType) + models.py # Data models (SystemInfo, CpuInfo, MemoryInfo, etc.) + parsers.py # Output parsing (ps, free, lsblk, ss, etc.) + formatters.py # Human-readable output formatting + audit.py # Audit logging with sensitive field redaction + mcp_app.py # MCP app UI constants + logging_config.py # Structured logging setup + __main__.py # CLI entry point + + tools/ # MCP tool definitions + system_info.py # get_system/cpu/memory/disk/hardware_information + services.py # list_services, get_service_status, get_service_logs + network.py # get_network_interfaces/connections, get_listening_ports + processes.py # list_processes, get_process_info + storage.py # list_block_devices/directories/files, read_file + logs.py # get_journal_logs, read_log_file + run_script.py # validate_script, run_script, run_script_with_confirmation, etc. + + connection/ + ssh.py # Singleton SSH connection manager with pooling + + gatekeeper/ + check_run_script.py # LLM-based script safety validation + + utils/ + decorators.py # Container detection, tool call logging + validation.py # Input validation helpers + format.py # Output formatting utilities + types.py # Type hint definitions + enum.py # Custom StrEnum + +tests/ # Mirrors src structure + conftest.py # Shared fixtures (mock SSHExecutor, MCP client) + tools/ # Tool-specific tests + parsers/ # Parser tests + connection/ # SSH tests + gatekeeper/ # Gatekeeper tests + functional/ # Integration tests + +mcp-app/ # React TypeScript UI for interactive script approval +docs/ # MkDocs documentation site +scripts/ # Utility scripts (tool doc generation, etc.) +``` + +## Tools Overview + +### Fixed Tools (read-only, always available) + +| Category | Tools | +|----------|-------| +| System | `get_system_information`, `get_cpu_information`, `get_memory_information`, `get_disk_usage`, `get_hardware_information` | +| Services | `list_services`, `get_service_status`, `get_service_logs` | +| Network | `get_network_interfaces`, `get_network_connections`, `get_listening_ports` | +| Processes| `list_processes`, `get_process_info` | +| Storage | `list_block_devices`, `list_directories`, `list_files`, `read_file` | +| Logs | `get_journal_logs`, `read_log_file` | + +### Guarded Command Execution (optional, `LINUX_MCP_TOOLSET=run_script|both`) + +`validate_script`, `run_script`, `run_script_with_confirmation`, `run_script_interactive`, `get_execution_state`, `reject_script` + +Every tool accepts an optional `host` parameter for remote execution via SSH. + +## Key Configuration (`LINUX_MCP_` prefix) + +| Variable | Purpose | Default | +|----------|---------|---------| +| `TOOLSET` | `fixed`, `run_script`, or `both` | `fixed` | +| `TRANSPORT` | `stdio`, `http`, or `streamable-http` | `stdio` | +| `GATEKEEPER_MODEL` | LLM model for script validation | required for `run_script` | +| `USER` | Default SSH username | - | +| `SSH_KEY_PATH` | Path to SSH private key | - | +| `VERIFY_HOST_KEYS` | SSH host key verification | `true` | +| `COMMAND_TIMEOUT` | Command timeout (seconds) | `30` | +| `ALLOWED_LOG_PATHS` | Comma-separated allowlist for `read_log_file` | none | +| `MAX_FILE_READ_BYTES` | Max file size for `read_file` | 1MB | +| `LOG_LEVEL` | `DEBUG`, `INFO`, `WARNING` | `INFO` | + +Full reference: `docs/config-reference.md` + +## Tech Stack + +- **Python 3.10+** with async/await throughout +- **FastMCP** (^2.14.4) - MCP server framework (currently on 2.x; upgrade to 3.x planned) +- **asyncssh** - SSH with key-based auth, connection pooling +- **Pydantic** - Config and data validation +- **LiteLLM** - Gatekeeper model inference +- **ruff** / **pyright** / **pytest** - Lint, types, tests +- **uv** - Package manager ## Rules @@ -33,6 +160,9 @@ make verify # All checks (required before commit) **Security (Critical):** - All tools must be read-only with `readOnlyHint=True` - Validate all input, use allowlists for file paths, sanitize shell params +- SSH key-based auth only (no passwords) +- Host key verification enabled by default +- Sensitive fields redacted in audit logs ## Adding Tools @@ -52,6 +182,6 @@ Types: `feat`, `fix`, `docs`, `test`, `refactor`, `perf`, `chore` ## Docs -Full details: `docs/contributing.md` | Architecture: `docs/architecture.md` +Full details: `docs/contributing.md` | Architecture: `docs/architecture.md` | Security: `docs/security.md` Tool docs under `docs/tools/` are auto-generated — run `uv run python scripts/generate_tool_docs.py` after adding or modifying tools. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..3cdcc68f --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,13 @@ +# Core Rules + +1. Don’t assume. Don’t hide confusion. Surface tradeoffs. + +2. Minimum code that solves the problem. Nothing speculative. + +3. Touch only what you must. Clean up only your own mess. + +4. Define success criteria. Loop until verified. + +# Project Details + +Please check out ./AGENTS.md