From 6a54cb15742d1e19afafbc17c72ae7657aabed9c Mon Sep 17 00:00:00 2001 From: Joe S Date: Wed, 22 Apr 2026 15:30:16 -0700 Subject: [PATCH 1/3] add ai helper docs for agentic dev --- .agents/architecture.md | 162 ++++++++++++++++++++++++++++++++ .agents/review.md | 83 ++++++++++++++++ .github/copilot-instructions.md | 1 + AGENTS.md | 105 +++++++++++++++++++++ CLAUDE.md | 5 + 5 files changed, 356 insertions(+) create mode 100644 .agents/architecture.md create mode 100644 .agents/review.md create mode 100644 .github/copilot-instructions.md create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/.agents/architecture.md b/.agents/architecture.md new file mode 100644 index 00000000..6bf26553 --- /dev/null +++ b/.agents/architecture.md @@ -0,0 +1,162 @@ +# AI Architecture And Repo Context + +This document provides repo-specific context for AI agents working in `mcp-clickhouse`. + +`AGENTS.md` is the operational source of truth. This file is required reading before substantial code changes, but it does not override `AGENTS.md`. + +## Repository Overview + +`mcp-clickhouse` is an MCP (Model Context Protocol) server that exposes ClickHouse and chDB to MCP clients such as Claude Desktop. It is built on `FastMCP` and uses `clickhouse-connect` as the ClickHouse HTTP driver. The package is published to PyPI as `mcp-clickhouse` and is distributed as a container image. + +Top-level areas that matter most: + +```text +mcp_clickhouse/ + mcp_server.py FastMCP server, tool registration, query execution, pagination, health check + mcp_env.py Environment-driven config (ClickHouseConfig, ChDBConfig, MCPServerConfig) with singletons + main.py Entry point. Resolves transport and starts the server + mcp_middleware_hook.py Loads user middleware from MCP_MIDDLEWARE_MODULE + chdb_prompt.py Prompt text returned by the chdb_initial_prompt prompt +tests/ pytest suite. Most tests expect a live ClickHouse on localhost +test-services/ docker-compose for a local ClickHouse for test and development +example_middleware.py Reference middleware module for MCP_MIDDLEWARE_MODULE +fastmcp.json FastMCP project manifest pointing at mcp_clickhouse/mcp_server.py +Dockerfile Container image build +``` + +## Core Invariants + +### The tool surface is the public API + +This server is an MCP server. The protocol surface is what users and AI clients depend on. Treat the following as public: + +- Tool names: `run_query`, `list_databases`, `list_tables`, `run_chdb_select_query`. +- Tool argument names, types, defaults, and docstrings (docstrings become tool descriptions visible to clients). +- Tool return shapes, including: + - `list_tables` returns `{"tables": [...], "next_page_token": str | None, "total_tables": int}`. + - `run_query` returns `{"columns": [...], "rows": [...]}` on success or `{"status": "error", "message": str}` on handled failure. + - `run_chdb_select_query` returns a list of row dicts on success or `{"status": "error", "message": str}` on failure. + - `list_databases` returns a JSON-encoded string (intentional, existing contract). +- Prompt names such as `chdb_initial_prompt`. +- The `/health` custom route and its status code contract (`200` healthy, `503` unhealthy, `401` unauthorized when auth is enabled). + +Small internal refactors can still produce breaking changes at this surface. + +### Environment variables are public + +All documented `CLICKHOUSE_*`, `CHDB_*`, `CLICKHOUSE_MCP_*`, and `MCP_MIDDLEWARE_MODULE` variables are public. Names, defaults, and semantics are compatibility commitments. Changes here require README updates and clear intent. + +When adding new configuration, thread it through `mcp_clickhouse/mcp_env.py` rather than reading `os.environ` in tool code. + +### Safety defaults are intentional + +Three layered safety defaults ship on purpose: + +1. Queries run with `readonly=1` unless `CLICKHOUSE_ALLOW_WRITE_ACCESS=true`. +2. Even with writes enabled, destructive statements (DROP TABLE, DROP DATABASE, DROP VIEW, DROP DICTIONARY, TRUNCATE TABLE) are rejected unless `CLICKHOUSE_ALLOW_DROP=true`. The check lives in `_validate_query_for_destructive_ops` as a regex scan. +3. HTTP and SSE transports require an auth token via `CLICKHOUSE_MCP_AUTH_TOKEN` unless `CLICKHOUSE_MCP_AUTH_DISABLED=true` is set explicitly. + +Do not relax these defaults casually. When touching `get_readonly_setting`, `_validate_query_for_destructive_ops`, or the auth wiring in `mcp_server.py`, keep the behavior matrix intact and update tests. + +### ClickHouse and chDB are two independently optional backends + +Either, both, or (intentionally) neither can be enabled: + +- `CLICKHOUSE_ENABLED` (default `true`) gates `list_databases`, `list_tables`, and `run_query`. +- `CHDB_ENABLED` (default `false`) gates `run_chdb_select_query` and `chdb_initial_prompt`. +- chDB requires the `chdb` optional extra. If `chdb` is not installed, the server logs a warning and skips chDB tool registration rather than crashing. Preserve that behavior: do not make `chdb` a hard import at module load. +- The `/health` endpoint accounts for all combinations and returns `503` if both backends are effectively disabled or unreachable. + +Changes to backend enablement should keep all four combinations working. + +### Configuration is a singleton + +`get_config()`, `get_chdb_config()`, and `get_mcp_config()` return cached instances. The cache is process-wide. + +- In tests, use `monkeypatch.setenv` plus patches that target the config accessors, or reset the global via the existing fixtures. Do not rely on reimporting modules to re-read env vars. +- `ClickHouseConfig.__init__` validates required variables only when the backend is enabled. Preserve this. It is why the server can run in chDB-only mode without `CLICKHOUSE_HOST`. + +### Context-based client config overrides + +`create_clickhouse_client` merges per-request overrides from the FastMCP context under `CLIENT_CONFIG_OVERRIDES_KEY` on top of the base config. This is how middleware injects per-request routing, tenant overrides, and timeouts. + +- Non-dict values under this key are ignored with a warning. Keep that behavior. +- When outside a request context (for example, at startup or in unit tests), `get_context()` raises `RuntimeError` and the code falls back to the base config. Preserve this fallback. + +### Query execution runs through a bounded thread pool + +`QUERY_EXECUTOR` is a `ThreadPoolExecutor(max_workers=10)` registered for `atexit` shutdown. Both `run_query` and `run_chdb_select_query` submit work to it and enforce `CLICKHOUSE_MCP_QUERY_TIMEOUT`. + +- The timeout is applied at the Python level via `Future.result(timeout=...)`. The query is not canceled server-side. Be aware of that if you touch cancellation or timeout logic. +- Exceeding the pool size under load queues work. If you raise throughput expectations, think about whether the pool size or the model needs to change. + +### Pagination state lives in a TTL cache + +`table_pagination_cache` is a `cachetools.TTLCache(maxsize=100, ttl=3600)`. Tokens are UUIDs. + +- Tokens are invalidated across filter changes (`database`, `like`, `not_like`, `include_detailed_columns`). The server logs a warning and restarts from the beginning. Preserve that defensive behavior. +- The cache is process-local. It is not safe to rely on pagination state surviving server restarts or working across replicas. + +### Middleware is user-loadable code + +`MCP_MIDDLEWARE_MODULE` lets users inject arbitrary middleware by module name. The server calls `module.setup_middleware(mcp)`. + +- The loader lives in `mcp_middleware_hook.py`. It logs and reraises import errors. +- Documented FastMCP hooks (`on_call_tool`, `on_request`, `on_read_resource`, and siblings) are part of the user contract. Do not paper over FastMCP hook contract changes silently. +- `example_middleware.py` is the reference for docs and tests. Keep it runnable. + +### TLS is handled through truststore by default + +`mcp_clickhouse/__init__.py` calls `truststore.inject_into_ssl()` at import time unless `MCP_CLICKHOUSE_TRUSTSTORE_DISABLE=1` is set. This is why the server trusts system certificates out of the box. Do not move or remove this without understanding the implications for users on corporate networks. + +## Compatibility Matrix + +Keep these axes in mind when evaluating change risk: + +- Python `3.10+` (the declared floor in `pyproject.toml`). CI currently exercises Python `3.13`. +- ClickHouse server: CI runs against `clickhouse/clickhouse-server:24.10`. Behavior should stay reasonable across recent ClickHouse versions. +- `fastmcp` `>=2.0.0,<3.0.0`. +- `clickhouse-connect` `>=0.8.16`. +- Transports: `stdio`, `http`, `sse`. Most users are on `stdio`. HTTP and SSE are exercised when authentication or the health endpoint matter. +- Optional extras: bare install (no chDB) must keep working. `chdb` extra is exercised in CI with `CHDB_ENABLED=true`. +- Container image: the Dockerfile is built in CI and the image must at least import cleanly and start under the default command. + +## Performance And Resource Considerations + +The server is not a hot path like a driver, but a few areas matter: + +- `list_tables` makes one query per table for detailed column metadata. `include_detailed_columns=False` exists specifically for large schemas. Preserve that escape hatch and do not regress the batching in `get_paginated_table_data`. +- The query thread pool caps concurrency. Tool behavior under timeouts must stay predictable. +- Avoid per-row Python overhead in anything that touches large result sets. The server returns raw rows from `clickhouse-connect` without rebuilding them, which is intentional. + +## Testing Layout + +Tests live in `tests/`. Most expect a live ClickHouse on `localhost:8123`. + +- `test_tool.py`, `test_mcp_server.py`, `test_pagination.py` exercise tools against a real server. +- `test_chdb_tool.py` and `test_optional_chdb.py` exercise chDB paths. chDB ones require `CHDB_ENABLED=true` and the extra. +- `test_config_interface.py` and `test_auth_config.py` are pure config tests with `monkeypatch`. +- `test_middleware.py` and `test_context_config_override.py` exercise the middleware hook and the per-request config override path. These use mocks heavily. + +When adding a test: + +- Prefer adding to an existing file when the feature fits. +- If you need a new fixture, check `test_mcp_server.py` first. Its async `Client` fixtures are the canonical way to drive the MCP surface end to end. +- Use `load_dotenv()` as existing tests do so local `.env` settings are picked up. + +## Ad Hoc Validation Expectations + +For changes that touch query execution, destructive-op gating, read-only enforcement, auth, the health endpoint, middleware loading, or pagination, do not rely only on static reasoning. + +At minimum: + +- Run targeted pytest coverage for the affected files. +- For transport or auth changes, run the server locally under HTTP and hit `/health` with and without the `Authorization: Bearer` header. +- For query behavior changes, validate against a real local ClickHouse started from `test-services/docker-compose.yaml`. +- For chDB changes, validate with the `chdb` extra installed and `CHDB_ENABLED=true`. + +## How To Use This Doc + +Use this file to understand what is structurally important in the repo before changing code. + +Use `.agents/review.md` when the task is specifically code review, review feedback, or patch analysis. diff --git a/.agents/review.md b/.agents/review.md new file mode 100644 index 00000000..0f6e12e1 --- /dev/null +++ b/.agents/review.md @@ -0,0 +1,83 @@ +# AI Review Guide + +This document is for AI-assisted code review, patch review, and PR analysis in this repository. + +Read `AGENTS.md` first. If the review touches substantive code paths, read `.agents/architecture.md` before reviewing. + +## Review Priorities + +Prioritize findings in this order: + +1. Correctness bugs +2. Safety regressions (read-only enforcement, destructive-op gating, HTTP/SSE authentication) +3. Regressions in observable behavior (tool names, tool argument shapes, tool return shapes, prompt names, environment variable semantics, `/health` contract) +4. Public API and compatibility risk +5. Backend-enablement regressions across the ClickHouse and chDB combinations +6. Packaging and optional dependency regressions, especially around the `chdb` extra +7. Resource and timeout behavior (thread pool, query timeout, pagination cache) +8. Missing or weak tests +9. Style and nits + +## Repo-Specific Review Checklist + +When reviewing a change, explicitly check whether it affects: + +- tool names, argument names, argument defaults, or return shapes +- prompt names or prompt registration +- environment variable names, defaults, or semantics (update `README.md` if so) +- read-only enforcement in `get_readonly_setting` and `build_query_settings` +- destructive-op detection in `_validate_query_for_destructive_ops` (regex breadth, case handling, newline handling, new DDL forms) +- HTTP and SSE authentication wiring and the token-or-disabled invariant +- `/health` response contract across the four backend-enablement combinations (ClickHouse on or off, chDB on or off) +- behavior when the `chdb` extra is missing. The server should warn and skip registration, not crash +- config singletons and whether a code path mutates env vars without resetting the singleton +- pagination token lifecycle (creation, reuse across filter changes, TTL eviction) +- query thread pool behavior and the `CLICKHOUSE_MCP_QUERY_TIMEOUT` contract +- context-based client config overrides in `create_clickhouse_client` and non-dict handling +- middleware loader behavior, including error handling and the `setup_middleware` contract +- compatibility expectations for Python, ClickHouse, `fastmcp`, and `clickhouse-connect` versions covered in CI + +For tool-surface changes, confirm the tests exercise the behavior through the MCP client (`fastmcp.Client`) and not only the underlying function. + +## What Good Review Feedback Looks Like + +- Lead with findings, not summary. +- Order findings by severity. +- Use `file:line` references. +- Be explicit about impact. +- Call out what could break for real MCP clients and users (Claude Desktop configs, existing tool arguments, existing env vars). +- Distinguish confirmed issues from inferred risk. + +If no material issues are found, say that explicitly and mention any residual testing or compatibility gaps (for example, chDB path not exercised, HTTP transport not smoke-tested, safety matrix not re-validated). + +When a finding or assumption depends on library or server behavior that may have changed, verify it against the upstream source (ClickHouse, `clickhouse-connect`, `FastMCP`, `chDB`) rather than relying on memory. Note the version or commit you checked against. Flag any claim in the diff that looks version-sensitive and was not verified. + +## Preferred Review Output + +Use a structure like this: + +1. Findings, ordered by severity +2. Open questions or assumptions +3. Brief change summary, only if useful + +Each finding should answer: + +- what is wrong +- why it matters +- who or what it could break +- what evidence in the diff or repo context supports the concern + +These points should be brief but factual and accurate. + +## Review Closing Checklist + +Before saying a change looks good, make sure you understand: + +- whether the MCP tool surface (names, arguments, returns) changed, intentionally or not +- whether any environment variable behavior changed and whether `README.md` reflects it +- whether safety defaults still hold: read-only by default, DROP gated behind `CLICKHOUSE_ALLOW_DROP`, HTTP/SSE auth required unless explicitly disabled +- whether the change holds up across the ClickHouse-on, ClickHouse-off, chDB-on, chDB-off matrix +- whether the `chdb` extra still being optional is respected +- whether transport behavior still works for `stdio`, `http`, and `sse` +- whether tests target the right layer (unit config tests with `monkeypatch`, tool tests through a live ClickHouse, or MCP-level tests via `fastmcp.Client`) +- whether any important validation still has not been run, for example a local server round trip or a container-image smoke test diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..b4d58787 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1 @@ +Use the review instructions in .agents/review.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..b590a429 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,105 @@ +# Agent Instructions + +`AGENTS.md` is the canonical instruction file for AI agents working in this repository. If another agent-facing file disagrees with this one, this file wins. + +Required reading: + +- Before making substantial code changes, read `.agents/architecture.md`. +- Before doing code review, review feedback, or PR analysis, read `.agents/review.md`. + +Do not treat those docs as replacements for this file. They are required reference material. This file remains the source of truth for agent behavior. + +## Role + +Act like an experienced maintainer of a public MCP server that fronts a database. + +- Be opinionated from the perspective of a Python, MCP, and ClickHouse expert. +- Favor best practices, but stay practical. +- Do not engage in sycophancy. +- Think about the tool surface as something an AI client will drive, not just a human. +- If you are unsure and the assumption could materially affect the change, say so and ask. + +## Working Rules + +- Understand the full local context before changing code. +- Keep changes small, safe, and directly tied to the task. +- Do not over-engineer. +- Preserve existing conventions unless there is a strong reason not to. +- Preserve backward compatibility for tool names, tool argument shapes, tool return shapes, environment variable names, and environment variable semantics by default. +- Treat safety defaults as intentional. Read-only mode, destructive-op gating, and HTTP/SSE authentication all default to the safer setting on purpose. Do not relax them casually. +- When a behavior depends on an environment variable, thread it through `mcp_clickhouse/mcp_env.py` rather than reading `os.environ` in tool code. +- Use double quotes when writing new Python code. +- Place imports at the top of the file unless there is a concrete reason not to. +- Write idiomatic Python. + +## Tooling And Validation + +- Use `uv` for package management, for example `uv sync --all-extras --dev` or `uv pip install `. +- Run formatting and linting with `ruff` (configured for `line-length = 100` in `pyproject.toml`). +- Run tests with `pytest` via `uv run pytest tests`. +- Prefer `rg` over slower text search tools when inspecting the repo. +- `gh` is available for GitHub inspection when needed. + +## Repo Workflow + +- Local ClickHouse for tests is started with `docker compose -f test-services/docker-compose.yaml up -d`. If a local server is needed and unavailable, tell the user rather than guessing around it. +- The local compose file uses `CLICKHOUSE_USER=default`, `CLICKHOUSE_PASSWORD=clickhouse`, non-secure HTTP on port `8123`. CI uses the `default` user with an empty password. Match whichever environment you are in rather than hardcoding. +- chDB features live behind the `chdb` optional extra. Tests that exercise chDB need `CHDB_ENABLED=true` and the extra installed: `uv run --extra chdb pytest -v tests/test_chdb_tool.py`. +- Reuse existing fixtures and patterns (`load_dotenv()`, the `Client` fixtures in `test_mcp_server.py`, pagination helpers reused by `test_pagination.py`) instead of inventing new ones. + +## Server And Protocol Behavior Is Authoritative + +When in doubt: + +- For ClickHouse server behavior, including how a query setting takes effect, how readonly modes differ, how an error is produced, or how a type is serialized, consult the ClickHouse server source at `https://github.com/ClickHouse/ClickHouse`. +- For `clickhouse-connect` client behavior, consult `https://github.com/ClickHouse/clickhouse-connect`. That driver is a direct dependency and many MCP behaviors depend on its exact semantics. +- For MCP protocol and `FastMCP` semantics, consult `https://github.com/jlowin/fastmcp`. When tool registration, middleware hooks, authentication providers, or transport behavior matters, go read the framework. + +Do not guess from this repo alone and do not assume documentation is current. + +## Currency Of Information + +If a fact could be version-sensitive (library API, server behavior, env var semantics, recent bug fixes, new MCP protocol features), verify it against the upstream source rather than from memory. Prefer a live lookup over a confident recall. + +- Use web search or fetch tools, or `gh` against the upstream repo, to check the current state of ClickHouse, `clickhouse-connect`, `FastMCP`, and `chDB`. +- When you cite a behavior, note the version or commit you checked against so reviewers can re-verify. +- If you cannot confirm the current behavior and the assumption matters, say so explicitly instead of proceeding. + +## Change Style + +- Fix the real problem, not a nearby symptom. +- Do not bundle cosmetic cleanup into unrelated changes. +- Do not add dependencies without a strong reason. New runtime deps should fit in the existing minimal dependency set in `pyproject.toml`. +- Do not add abstractions for hypothetical future needs. +- If a workaround papers over a deeper issue (for example, a `clickhouse-connect` bug or a `FastMCP` quirk), say so plainly. + +## Tool Surface Is Public + +The MCP tool surface is the public API of this project. Treat the following as public: + +- Tool names: `run_query`, `list_databases`, `list_tables`, `run_chdb_select_query`. +- Tool argument names, types, and defaults. +- Tool return shapes, including dict keys (`tables`, `next_page_token`, `total_tables`, `columns`, `rows`), list element shapes, and error shapes (`status`, `message`). +- Prompt names, for example `chdb_initial_prompt`. +- The `/health` HTTP route and its response contract. + +Small internal refactors can still produce breaking behavior at the tool surface. When in doubt, preserve the existing shape. + +## Environment Variables Are Public + +Environment variable names and semantics are also public surface. That includes the `CLICKHOUSE_*`, `CHDB_*`, `CLICKHOUSE_MCP_*`, and `MCP_MIDDLEWARE_MODULE` names and their documented defaults. Renames, default changes, and removals are breaking changes. Update `README.md` alongside any intentional change. + +## Writing Style + +- Use only characters that are easy to reproduce on an American US keyboard. +- Use `->` for arrows. +- Do not use em dashes, en dashes, or smart quotes. +- Keep punctuation natural and simple. Prefer commas or periods. +- Limit parentheses. +- Use single spaces between sentences. + +## Test Data + +- Do not use `42` as the generic representative integer in tests. +- Do not use names like `alice` or `bob` as generic placeholders in new tests. Existing tests use them and do not need to be churned. +- Prefer values like `13`, `79`, `user_1`, and `user_2`, or similarly neutral domain-appropriate values. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..4808547c --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,5 @@ +# Claude Instructions + +This repository uses `AGENTS.md` as the canonical AI instruction file. + +Read `AGENTS.md` and follow it. Also read any docs it requires, especially `.agents/architecture.md` for substantial code changes and `.agents/review.md` for code review work. From 315133b0df932158fdbafdd8a4848ccba6f6b88d Mon Sep 17 00:00:00 2001 From: Joe S Date: Wed, 29 Apr 2026 12:55:07 -0700 Subject: [PATCH 2/3] refactor ai helper docs --- .agents/architecture.md | 152 ++++++------------------ .agents/review.md | 83 ------------- .agents/skills/review/SKILL.md | 79 ++++++++++++ .agents/skills/testing/SKILL.md | 111 +++++++++++++++++ .agents/skills/upstream-verify/SKILL.md | 61 ++++++++++ .claude/agents/behavior-prober.md | 70 +++++++++++ .github/copilot-instructions.md | 2 +- AGENTS.md | 109 ++++------------- CLAUDE.md | 9 +- mcp_clickhouse/__init__.py | 3 + mcp_clickhouse/mcp_env.py | 16 ++- mcp_clickhouse/mcp_server.py | 12 +- 12 files changed, 413 insertions(+), 294 deletions(-) delete mode 100644 .agents/review.md create mode 100644 .agents/skills/review/SKILL.md create mode 100644 .agents/skills/testing/SKILL.md create mode 100644 .agents/skills/upstream-verify/SKILL.md create mode 100644 .claude/agents/behavior-prober.md diff --git a/.agents/architecture.md b/.agents/architecture.md index 6bf26553..e9ebe96b 100644 --- a/.agents/architecture.md +++ b/.agents/architecture.md @@ -1,14 +1,12 @@ -# AI Architecture And Repo Context +# Architecture and repo context -This document provides repo-specific context for AI agents working in `mcp-clickhouse`. +Read this before substantial code changes. For test commands and validation, see `.agents/skills/testing/SKILL.md`. For review work, see `.agents/skills/review/SKILL.md`. For library or server claims, see `.agents/skills/upstream-verify/SKILL.md`. -`AGENTS.md` is the operational source of truth. This file is required reading before substantial code changes, but it does not override `AGENTS.md`. +## What this is -## Repository Overview +`mcp-clickhouse` is an MCP server that exposes ClickHouse and chDB to MCP clients (Claude Desktop, Cursor, etc.). It is built on `FastMCP` and uses `clickhouse-connect` as the HTTP driver. The package ships to PyPI as `mcp-clickhouse` and as a container image. -`mcp-clickhouse` is an MCP (Model Context Protocol) server that exposes ClickHouse and chDB to MCP clients such as Claude Desktop. It is built on `FastMCP` and uses `clickhouse-connect` as the ClickHouse HTTP driver. The package is published to PyPI as `mcp-clickhouse` and is distributed as a container image. - -Top-level areas that matter most: +## Layout ```text mcp_clickhouse/ @@ -18,145 +16,65 @@ mcp_clickhouse/ mcp_middleware_hook.py Loads user middleware from MCP_MIDDLEWARE_MODULE chdb_prompt.py Prompt text returned by the chdb_initial_prompt prompt tests/ pytest suite. Most tests expect a live ClickHouse on localhost -test-services/ docker-compose for a local ClickHouse for test and development +test-services/ docker-compose for local ClickHouse for test and development example_middleware.py Reference middleware module for MCP_MIDDLEWARE_MODULE fastmcp.json FastMCP project manifest pointing at mcp_clickhouse/mcp_server.py Dockerfile Container image build ``` -## Core Invariants - -### The tool surface is the public API - -This server is an MCP server. The protocol surface is what users and AI clients depend on. Treat the following as public: - -- Tool names: `run_query`, `list_databases`, `list_tables`, `run_chdb_select_query`. -- Tool argument names, types, defaults, and docstrings (docstrings become tool descriptions visible to clients). -- Tool return shapes, including: - - `list_tables` returns `{"tables": [...], "next_page_token": str | None, "total_tables": int}`. - - `run_query` returns `{"columns": [...], "rows": [...]}` on success or `{"status": "error", "message": str}` on handled failure. - - `run_chdb_select_query` returns a list of row dicts on success or `{"status": "error", "message": str}` on failure. - - `list_databases` returns a JSON-encoded string (intentional, existing contract). -- Prompt names such as `chdb_initial_prompt`. -- The `/health` custom route and its status code contract (`200` healthy, `503` unhealthy, `401` unauthorized when auth is enabled). +## Cross-cutting invariants -Small internal refactors can still produce breaking changes at this surface. +Function-level invariants (singleton config caching, context-override handling, truststore inject, thread-pool timeout semantics) live in docstrings on the relevant code. The items below are the cross-cutting ones that do not belong in any single function. -### Environment variables are public +### Layered safety defaults -All documented `CLICKHOUSE_*`, `CHDB_*`, `CLICKHOUSE_MCP_*`, and `MCP_MIDDLEWARE_MODULE` variables are public. Names, defaults, and semantics are compatibility commitments. Changes here require README updates and clear intent. - -When adding new configuration, thread it through `mcp_clickhouse/mcp_env.py` rather than reading `os.environ` in tool code. - -### Safety defaults are intentional - -Three layered safety defaults ship on purpose: +Three defaults ship at the safer setting on purpose: 1. Queries run with `readonly=1` unless `CLICKHOUSE_ALLOW_WRITE_ACCESS=true`. -2. Even with writes enabled, destructive statements (DROP TABLE, DROP DATABASE, DROP VIEW, DROP DICTIONARY, TRUNCATE TABLE) are rejected unless `CLICKHOUSE_ALLOW_DROP=true`. The check lives in `_validate_query_for_destructive_ops` as a regex scan. +2. Even with writes enabled, destructive statements (`DROP TABLE`, `DROP DATABASE`, `DROP VIEW`, `DROP DICTIONARY`, `TRUNCATE TABLE`) are rejected unless `CLICKHOUSE_ALLOW_DROP=true`. The check is a regex scan in `_validate_query_for_destructive_ops`. 3. HTTP and SSE transports require an auth token via `CLICKHOUSE_MCP_AUTH_TOKEN` unless `CLICKHOUSE_MCP_AUTH_DISABLED=true` is set explicitly. -Do not relax these defaults casually. When touching `get_readonly_setting`, `_validate_query_for_destructive_ops`, or the auth wiring in `mcp_server.py`, keep the behavior matrix intact and update tests. +When touching `get_readonly_setting`, `_validate_query_for_destructive_ops`, or the auth wiring in `mcp_server.py`, keep the behavior matrix intact and update tests. -### ClickHouse and chDB are two independently optional backends +### Two independently optional backends -Either, both, or (intentionally) neither can be enabled: +ClickHouse and chDB can each be enabled or disabled. All four combinations must keep working: -- `CLICKHOUSE_ENABLED` (default `true`) gates `list_databases`, `list_tables`, and `run_query`. +- `CLICKHOUSE_ENABLED` (default `true`) gates `list_databases`, `list_tables`, `run_query`. - `CHDB_ENABLED` (default `false`) gates `run_chdb_select_query` and `chdb_initial_prompt`. -- chDB requires the `chdb` optional extra. If `chdb` is not installed, the server logs a warning and skips chDB tool registration rather than crashing. Preserve that behavior: do not make `chdb` a hard import at module load. -- The `/health` endpoint accounts for all combinations and returns `503` if both backends are effectively disabled or unreachable. - -Changes to backend enablement should keep all four combinations working. - -### Configuration is a singleton - -`get_config()`, `get_chdb_config()`, and `get_mcp_config()` return cached instances. The cache is process-wide. - -- In tests, use `monkeypatch.setenv` plus patches that target the config accessors, or reset the global via the existing fixtures. Do not rely on reimporting modules to re-read env vars. -- `ClickHouseConfig.__init__` validates required variables only when the backend is enabled. Preserve this. It is why the server can run in chDB-only mode without `CLICKHOUSE_HOST`. - -### Context-based client config overrides - -`create_clickhouse_client` merges per-request overrides from the FastMCP context under `CLIENT_CONFIG_OVERRIDES_KEY` on top of the base config. This is how middleware injects per-request routing, tenant overrides, and timeouts. - -- Non-dict values under this key are ignored with a warning. Keep that behavior. -- When outside a request context (for example, at startup or in unit tests), `get_context()` raises `RuntimeError` and the code falls back to the base config. Preserve this fallback. - -### Query execution runs through a bounded thread pool +- chDB requires the `chdb` optional extra. If the package is missing, the server warns and skips chDB tool registration. Do not make `chdb` a hard import at module load. +- `/health` accounts for all combinations. It returns `503` if both backends are effectively disabled or unreachable. -`QUERY_EXECUTOR` is a `ThreadPoolExecutor(max_workers=10)` registered for `atexit` shutdown. Both `run_query` and `run_chdb_select_query` submit work to it and enforce `CLICKHOUSE_MCP_QUERY_TIMEOUT`. +### Tool surface and env vars are public -- The timeout is applied at the Python level via `Future.result(timeout=...)`. The query is not canceled server-side. Be aware of that if you touch cancellation or timeout logic. -- Exceeding the pool size under load queues work. If you raise throughput expectations, think about whether the pool size or the model needs to change. +These are the contract. Treat all changes here as breaking and update `README.md` alongside any intentional change. `AGENTS.md` lists the surface explicitly. -### Pagination state lives in a TTL cache +### Pagination state is process-local -`table_pagination_cache` is a `cachetools.TTLCache(maxsize=100, ttl=3600)`. Tokens are UUIDs. - -- Tokens are invalidated across filter changes (`database`, `like`, `not_like`, `include_detailed_columns`). The server logs a warning and restarts from the beginning. Preserve that defensive behavior. -- The cache is process-local. It is not safe to rely on pagination state surviving server restarts or working across replicas. +`table_pagination_cache` is a `cachetools.TTLCache(maxsize=100, ttl=3600)`. Tokens are UUIDs. Tokens are invalidated across filter changes (`database`, `like`, `not_like`, `include_detailed_columns`); the server logs a warning and restarts from the beginning. Pagination state does not survive process restarts and is not safe across replicas. ### Middleware is user-loadable code -`MCP_MIDDLEWARE_MODULE` lets users inject arbitrary middleware by module name. The server calls `module.setup_middleware(mcp)`. - -- The loader lives in `mcp_middleware_hook.py`. It logs and reraises import errors. -- Documented FastMCP hooks (`on_call_tool`, `on_request`, `on_read_resource`, and siblings) are part of the user contract. Do not paper over FastMCP hook contract changes silently. -- `example_middleware.py` is the reference for docs and tests. Keep it runnable. +`MCP_MIDDLEWARE_MODULE` lets users inject arbitrary middleware by module name. The loader (`mcp_middleware_hook.py`) calls `module.setup_middleware(mcp)` and reraises import errors. Documented FastMCP hooks (`on_call_tool`, `on_request`, `on_read_resource`, etc.) are part of the user contract. `example_middleware.py` is the reference and must stay runnable. -### TLS is handled through truststore by default +## Performance and resource notes -`mcp_clickhouse/__init__.py` calls `truststore.inject_into_ssl()` at import time unless `MCP_CLICKHOUSE_TRUSTSTORE_DISABLE=1` is set. This is why the server trusts system certificates out of the box. Do not move or remove this without understanding the implications for users on corporate networks. +The server is not a hot path, but a few areas matter: -## Compatibility Matrix +- `list_tables` makes one query per table for detailed column metadata. `include_detailed_columns=False` is the escape hatch for large schemas. Preserve the batching in `get_paginated_table_data`. +- The query thread pool caps concurrency at 10. Tool behavior under timeouts must stay predictable. `CLICKHOUSE_MCP_QUERY_TIMEOUT` is enforced at the Python level via `Future.result(timeout=...)`; the query is not canceled server-side. +- Large result sets are returned raw from `clickhouse-connect` without rebuilding. Avoid per-row Python overhead. -Keep these axes in mind when evaluating change risk: +## Compatibility axes -- Python `3.10+` (the declared floor in `pyproject.toml`). CI currently exercises Python `3.13`. -- ClickHouse server: CI runs against `clickhouse/clickhouse-server:24.10`. Behavior should stay reasonable across recent ClickHouse versions. +- Python `3.10+`. CI exercises Python `3.13`. +- ClickHouse server: CI uses `clickhouse/clickhouse-server:24.10`. Behavior should stay reasonable across recent versions. - `fastmcp` `>=2.0.0,<3.0.0`. - `clickhouse-connect` `>=0.8.16`. -- Transports: `stdio`, `http`, `sse`. Most users are on `stdio`. HTTP and SSE are exercised when authentication or the health endpoint matter. -- Optional extras: bare install (no chDB) must keep working. `chdb` extra is exercised in CI with `CHDB_ENABLED=true`. -- Container image: the Dockerfile is built in CI and the image must at least import cleanly and start under the default command. - -## Performance And Resource Considerations - -The server is not a hot path like a driver, but a few areas matter: - -- `list_tables` makes one query per table for detailed column metadata. `include_detailed_columns=False` exists specifically for large schemas. Preserve that escape hatch and do not regress the batching in `get_paginated_table_data`. -- The query thread pool caps concurrency. Tool behavior under timeouts must stay predictable. -- Avoid per-row Python overhead in anything that touches large result sets. The server returns raw rows from `clickhouse-connect` without rebuilding them, which is intentional. - -## Testing Layout - -Tests live in `tests/`. Most expect a live ClickHouse on `localhost:8123`. - -- `test_tool.py`, `test_mcp_server.py`, `test_pagination.py` exercise tools against a real server. -- `test_chdb_tool.py` and `test_optional_chdb.py` exercise chDB paths. chDB ones require `CHDB_ENABLED=true` and the extra. -- `test_config_interface.py` and `test_auth_config.py` are pure config tests with `monkeypatch`. -- `test_middleware.py` and `test_context_config_override.py` exercise the middleware hook and the per-request config override path. These use mocks heavily. - -When adding a test: - -- Prefer adding to an existing file when the feature fits. -- If you need a new fixture, check `test_mcp_server.py` first. Its async `Client` fixtures are the canonical way to drive the MCP surface end to end. -- Use `load_dotenv()` as existing tests do so local `.env` settings are picked up. - -## Ad Hoc Validation Expectations - -For changes that touch query execution, destructive-op gating, read-only enforcement, auth, the health endpoint, middleware loading, or pagination, do not rely only on static reasoning. - -At minimum: - -- Run targeted pytest coverage for the affected files. -- For transport or auth changes, run the server locally under HTTP and hit `/health` with and without the `Authorization: Bearer` header. -- For query behavior changes, validate against a real local ClickHouse started from `test-services/docker-compose.yaml`. -- For chDB changes, validate with the `chdb` extra installed and `CHDB_ENABLED=true`. - -## How To Use This Doc +- Transports: `stdio`, `http`, `sse`. +- Optional extras: bare install (no chDB) must keep working. The `chdb` extra is exercised in CI. +- Container image: built in CI; must at least import and start under the default command. -Use this file to understand what is structurally important in the repo before changing code. +## When code and this doc disagree -Use `.agents/review.md` when the task is specifically code review, review feedback, or patch analysis. +The code wins. If you find a divergence while working, fix the doc in the same PR rather than letting it rot. diff --git a/.agents/review.md b/.agents/review.md deleted file mode 100644 index 0f6e12e1..00000000 --- a/.agents/review.md +++ /dev/null @@ -1,83 +0,0 @@ -# AI Review Guide - -This document is for AI-assisted code review, patch review, and PR analysis in this repository. - -Read `AGENTS.md` first. If the review touches substantive code paths, read `.agents/architecture.md` before reviewing. - -## Review Priorities - -Prioritize findings in this order: - -1. Correctness bugs -2. Safety regressions (read-only enforcement, destructive-op gating, HTTP/SSE authentication) -3. Regressions in observable behavior (tool names, tool argument shapes, tool return shapes, prompt names, environment variable semantics, `/health` contract) -4. Public API and compatibility risk -5. Backend-enablement regressions across the ClickHouse and chDB combinations -6. Packaging and optional dependency regressions, especially around the `chdb` extra -7. Resource and timeout behavior (thread pool, query timeout, pagination cache) -8. Missing or weak tests -9. Style and nits - -## Repo-Specific Review Checklist - -When reviewing a change, explicitly check whether it affects: - -- tool names, argument names, argument defaults, or return shapes -- prompt names or prompt registration -- environment variable names, defaults, or semantics (update `README.md` if so) -- read-only enforcement in `get_readonly_setting` and `build_query_settings` -- destructive-op detection in `_validate_query_for_destructive_ops` (regex breadth, case handling, newline handling, new DDL forms) -- HTTP and SSE authentication wiring and the token-or-disabled invariant -- `/health` response contract across the four backend-enablement combinations (ClickHouse on or off, chDB on or off) -- behavior when the `chdb` extra is missing. The server should warn and skip registration, not crash -- config singletons and whether a code path mutates env vars without resetting the singleton -- pagination token lifecycle (creation, reuse across filter changes, TTL eviction) -- query thread pool behavior and the `CLICKHOUSE_MCP_QUERY_TIMEOUT` contract -- context-based client config overrides in `create_clickhouse_client` and non-dict handling -- middleware loader behavior, including error handling and the `setup_middleware` contract -- compatibility expectations for Python, ClickHouse, `fastmcp`, and `clickhouse-connect` versions covered in CI - -For tool-surface changes, confirm the tests exercise the behavior through the MCP client (`fastmcp.Client`) and not only the underlying function. - -## What Good Review Feedback Looks Like - -- Lead with findings, not summary. -- Order findings by severity. -- Use `file:line` references. -- Be explicit about impact. -- Call out what could break for real MCP clients and users (Claude Desktop configs, existing tool arguments, existing env vars). -- Distinguish confirmed issues from inferred risk. - -If no material issues are found, say that explicitly and mention any residual testing or compatibility gaps (for example, chDB path not exercised, HTTP transport not smoke-tested, safety matrix not re-validated). - -When a finding or assumption depends on library or server behavior that may have changed, verify it against the upstream source (ClickHouse, `clickhouse-connect`, `FastMCP`, `chDB`) rather than relying on memory. Note the version or commit you checked against. Flag any claim in the diff that looks version-sensitive and was not verified. - -## Preferred Review Output - -Use a structure like this: - -1. Findings, ordered by severity -2. Open questions or assumptions -3. Brief change summary, only if useful - -Each finding should answer: - -- what is wrong -- why it matters -- who or what it could break -- what evidence in the diff or repo context supports the concern - -These points should be brief but factual and accurate. - -## Review Closing Checklist - -Before saying a change looks good, make sure you understand: - -- whether the MCP tool surface (names, arguments, returns) changed, intentionally or not -- whether any environment variable behavior changed and whether `README.md` reflects it -- whether safety defaults still hold: read-only by default, DROP gated behind `CLICKHOUSE_ALLOW_DROP`, HTTP/SSE auth required unless explicitly disabled -- whether the change holds up across the ClickHouse-on, ClickHouse-off, chDB-on, chDB-off matrix -- whether the `chdb` extra still being optional is respected -- whether transport behavior still works for `stdio`, `http`, and `sse` -- whether tests target the right layer (unit config tests with `monkeypatch`, tool tests through a live ClickHouse, or MCP-level tests via `fastmcp.Client`) -- whether any important validation still has not been run, for example a local server round trip or a container-image smoke test diff --git a/.agents/skills/review/SKILL.md b/.agents/skills/review/SKILL.md new file mode 100644 index 00000000..7527d967 --- /dev/null +++ b/.agents/skills/review/SKILL.md @@ -0,0 +1,79 @@ +--- +name: review +description: Use when reviewing pull requests, code changes, or patches in mcp-clickhouse. Covers severity ordering, repo-specific checks, and the closing checklist for approving a change. +--- + +# Review skill + +For substantive review, read `.agents/architecture.md` first so the invariants below have context. For test-running and validation commands, use the `testing` skill rather than reasoning from memory. + +## Severity order + +Order findings by impact, highest first: + +1. Correctness bugs. +2. Safety regressions: read-only enforcement, destructive-op gating, HTTP/SSE authentication. +3. Regressions in observable behavior: tool names, tool argument shapes, tool return shapes, prompt names, environment variable semantics, `/health` contract. +4. Public API and compatibility risk. +5. Backend-enablement regressions across the ClickHouse and chDB combinations. +6. Packaging and optional-dependency regressions, especially around the `chdb` extra. +7. Resource and timeout behavior: thread pool, query timeout, pagination cache. +8. Missing or weak tests. +9. Style and nits. + +## Repo-specific checklist + +When reviewing, explicitly check whether the change affects: + +- tool names, argument names, argument defaults, or return shapes. +- prompt names or prompt registration. +- environment variable names, defaults, or semantics. If yes, `README.md` should change too. +- read-only enforcement in `get_readonly_setting` and `build_query_settings`. +- destructive-op detection in `_validate_query_for_destructive_ops`. Watch the regex for breadth, case handling, newline handling, and new DDL forms. +- HTTP and SSE authentication wiring and the token-or-disabled invariant. +- `/health` response contract across the four backend-enablement combinations. +- behavior when the `chdb` extra is missing. The server should warn and skip registration, not crash. +- config singletons. Flag any code path that mutates env vars without resetting the singleton. +- pagination token lifecycle: creation, reuse across filter changes, TTL eviction. +- query thread pool behavior and the `CLICKHOUSE_MCP_QUERY_TIMEOUT` contract. +- context-based client config overrides in `create_clickhouse_client` and the non-dict warning path. +- middleware loader behavior, including error handling and the `setup_middleware` contract. +- compatibility expectations for Python, ClickHouse, `fastmcp`, and `clickhouse-connect` versions covered in CI. + +For tool-surface changes, confirm tests exercise the behavior through `fastmcp.Client`, not only the underlying function. + +## Verifying claims + +When a finding or assumption depends on library or server behavior that may have changed, use the `upstream-verify` skill to check it against the actual upstream source. Note the version or commit you checked. Flag any claim in the diff that looks version-sensitive and was not verified. + +## Output structure + +Use this shape: + +1. Findings, ordered by severity. +2. Open questions or assumptions. +3. Brief change summary, only if useful. + +Each finding should answer: + +- what is wrong +- why it matters +- who or what it could break (Claude Desktop configs, existing tool arguments, existing env vars, etc.) +- what evidence in the diff or repo context supports the concern + +Lead with findings, not summary. Use `file:line` references. Be explicit about impact. Distinguish confirmed issues from inferred risk. + +If no material issues are found, say so explicitly and mention any residual gaps: chDB path not exercised, HTTP transport not smoke-tested, safety matrix not re-validated, etc. + +## Closing checklist + +Before saying a change looks good, confirm you understand: + +- whether the MCP tool surface (names, arguments, returns) changed, intentionally or not. +- whether any environment variable behavior changed and whether `README.md` reflects it. +- whether safety defaults still hold: read-only by default, DROP gated behind `CLICKHOUSE_ALLOW_DROP`, HTTP/SSE auth required unless explicitly disabled. +- whether the change holds up across the ClickHouse-on, ClickHouse-off, chDB-on, chDB-off matrix. +- whether the `chdb` extra still being optional is respected. +- whether transport behavior still works for `stdio`, `http`, and `sse`. +- whether tests target the right layer: unit config tests with `monkeypatch`, tool tests through a live ClickHouse, MCP-level tests via `fastmcp.Client`. +- whether any important validation still has not been run, for example a local server round trip or a container-image smoke test. diff --git a/.agents/skills/testing/SKILL.md b/.agents/skills/testing/SKILL.md new file mode 100644 index 00000000..8d4d81c4 --- /dev/null +++ b/.agents/skills/testing/SKILL.md @@ -0,0 +1,111 @@ +--- +name: testing +description: Use when running, writing, debugging, or modifying tests in mcp-clickhouse. Covers exact uv commands, ClickHouse and chDB test setup, fixture patterns, and ad hoc validation expectations. +--- + +# Testing skill + +Tests live in `tests/`. Most expect a live ClickHouse on `localhost:8123`. Use the exact commands below rather than reaching for memory; the project standardizes on `uv` and `pytest`. + +## Bring up the local ClickHouse + +The compose file uses `CLICKHOUSE_USER=default`, `CLICKHOUSE_PASSWORD=clickhouse`, non-secure HTTP on port `8123`. + +```bash +docker compose -f test-services/docker-compose.yaml up -d +``` + +CI runs against `clickhouse/clickhouse-server:24.10` with the `default` user and an empty password. Match the environment you are in rather than hardcoding credentials. + +If a local server is required and is not available, say so to the user. Do not silently mock around it. + +## Sync dependencies + +```bash +uv sync --all-extras --dev +``` + +Use `uv pip install ` if you need a single package quickly. + +## Run the suite + +Full run: + +```bash +uv run pytest tests +``` + +Targeted file or test: + +```bash +uv run pytest tests/test_pagination.py +uv run pytest tests/test_mcp_server.py::test_run_query +``` + +Verbose, with stdout pass-through: + +```bash +uv run pytest -v -s tests/test_mcp_server.py +``` + +## chDB tests + +chDB lives behind the `chdb` optional extra. Tests that touch chDB need the extra installed and `CHDB_ENABLED=true`: + +```bash +CHDB_ENABLED=true uv run --extra chdb pytest -v tests/test_chdb_tool.py +``` + +`tests/test_optional_chdb.py` exercises the path where the `chdb` package is not installed. The server must warn and skip chDB tool registration in that case, not crash. Keep that guarantee intact. + +## Lint and format + +```bash +uv run ruff check +uv run ruff format +``` + +`pyproject.toml` configures `line-length = 100`. + +## Test layout reference + +- `test_tool.py`, `test_mcp_server.py`, `test_pagination.py` exercise tools against a real ClickHouse. +- `test_chdb_tool.py`, `test_optional_chdb.py` exercise chDB paths. +- `test_config_interface.py`, `test_auth_config.py` are pure config tests using `monkeypatch`. +- `test_middleware.py`, `test_context_config_override.py` exercise the middleware hook and the per-request config override path. Heavy on mocks. + +## Fixture and convention rules + +- Reuse existing fixtures. The async `Client` fixtures in `test_mcp_server.py` are the canonical way to drive the MCP surface end to end. +- Call `load_dotenv()` as existing tests do so local `.env` settings are picked up. +- For tool-surface changes, exercise the behavior through the MCP client (`fastmcp.Client`), not only the underlying function. +- Do not rely on reimporting modules to re-read env vars. Config accessors (`get_config`, `get_chdb_config`, `get_mcp_config`) are cached singletons. Use `monkeypatch.setenv` plus patches that target the accessors, or reset via the existing fixtures. +- Prefer adding to an existing test file when the feature fits the file's theme. + +## Test data conventions + +- Avoid `42` as the generic representative integer. +- Avoid `alice` and `bob` as placeholders in new tests. Existing tests use them and do not need churn. +- Prefer `13`, `79`, `user_1`, `user_2`, or other neutral domain-appropriate values. + +## Ad hoc validation expectations + +For changes that touch query execution, destructive-op gating, read-only enforcement, auth, the `/health` endpoint, middleware loading, or pagination, do not rely only on static reasoning. + +At minimum: + +- Run targeted pytest coverage for the affected files. +- For transport or auth changes, run the server locally under HTTP and hit `/health` with and without an `Authorization: Bearer` header. +- For query behavior changes, validate against a real local ClickHouse from `test-services/docker-compose.yaml`. +- For chDB changes, validate with the `chdb` extra installed and `CHDB_ENABLED=true`. + +## Backend-enablement matrix + +Many bugs hide in the corners of the four-cell matrix. When a change touches tool registration, `/health`, or config, check all combinations: + +| `CLICKHOUSE_ENABLED` | `CHDB_ENABLED` | Expected | +| --- | --- | --- | +| true (default) | false (default) | ClickHouse tools registered, chDB skipped | +| true | true | Both registered, chDB requires the `chdb` extra | +| false | true | chDB-only mode; ClickHouse config not required | +| false | false | `/health` returns 503; intentional misconfiguration | diff --git a/.agents/skills/upstream-verify/SKILL.md b/.agents/skills/upstream-verify/SKILL.md new file mode 100644 index 00000000..9dc8dc27 --- /dev/null +++ b/.agents/skills/upstream-verify/SKILL.md @@ -0,0 +1,61 @@ +--- +name: upstream-verify +description: Use when a claim, fix, or review finding depends on ClickHouse, clickhouse-connect, FastMCP, or chDB behavior that could be version-sensitive. Use this rather than relying on training memory for library or server semantics. +--- + +# Upstream verification skill + +Library and server behavior changes between versions. Memory of any of the projects below is likely stale. When a code change, bug claim, or review finding depends on how one of them actually behaves, verify against the upstream source rather than guessing. + +Trigger this skill when: + +- a query setting, readonly mode, error message, or type serialization claim depends on ClickHouse server behavior +- a connection, settings, or result-shape claim depends on `clickhouse-connect` driver semantics +- a tool registration, middleware hook, auth provider, or transport claim depends on `FastMCP` +- a chDB session, query, or in-process behavior claim depends on `chDB` +- the diff adjusts behavior tied to a specific upstream version and the diff does not say which version + +## Where to look + +| Concern | Source | +| --- | --- | +| ClickHouse server | https://github.com/ClickHouse/ClickHouse | +| `clickhouse-connect` (HTTP driver) | https://github.com/ClickHouse/clickhouse-connect | +| `FastMCP` (MCP server framework) | https://github.com/jlowin/fastmcp | +| `chDB` (in-process ClickHouse) | https://github.com/chdb-io/chdb | + +Treat the source repo as authoritative over docs, blog posts, or memory. Docs lag. + +## How to verify + +Pick the lightest tool that answers the question: + +- File-level inspection: `gh api repos///contents/?ref=` returns base64 content. +- Symbol search: `gh api search/code -q " repo:/"`. +- Diff between versions: `gh api repos///compare/...`. +- Changelog or release notes: `gh release view --repo /` or read `CHANGELOG.md` from the right ref. +- For chDB and FastMCP, the README and tests in the repo are usually the fastest proof. + +If the harness exposes web fetch or web search, those are valid too. The point is to ground the claim in something observable, not pin a specific tool. + +## Pinning what you checked + +When citing upstream behavior in a finding, code comment, or PR comment, note the version or commit you verified against. Future readers (and future agents) need that to re-check. A line like: + +> Verified against `clickhouse-connect@0.8.16` (commit ``): `client.server_settings` returns `Setting` objects with a `.value` attribute. + +is enough. + +## When you cannot verify + +If upstream is unreachable, pinned to a commit you cannot resolve, or the answer requires running the library, say so explicitly. Do not paper over it. Flag the unverified assumption in the diff or review and let a human resolve it. + +## Compatibility floors that already matter + +Keep these in mind when picking which version to verify against. The repo has to keep working across all of them. + +- Python `3.10+` (declared floor in `pyproject.toml`). CI exercises Python `3.13`. +- ClickHouse server: CI runs against `clickhouse/clickhouse-server:24.10`. Behavior should stay reasonable across recent ClickHouse versions. +- `fastmcp` `>=2.0.0,<3.0.0`. +- `clickhouse-connect` `>=0.8.16`. +- Transports: `stdio`, `http`, `sse`. HTTP and SSE matter most when authentication or `/health` is in scope. diff --git a/.claude/agents/behavior-prober.md b/.claude/agents/behavior-prober.md new file mode 100644 index 00000000..1ec8c1f4 --- /dev/null +++ b/.claude/agents/behavior-prober.md @@ -0,0 +1,70 @@ +--- +name: behavior-prober +description: Use after implementing or changing MCP server behavior when you want a thorough pre-submit confidence check that the change works correctly across realistic usage. Hand off the change and the area to probe. This agent will exercise it the way a careful developer would before merging, covering happy paths, the practical edges, common tool-call shapes, error paths, transport modes, and sync vs async where relevant. It returns a structured findings report and keeps the noise of throwaway scripts and test runs out of the main thread. Not a fuzzer and not a token sanity demo. +tools: Read, Bash, Grep, Glob, Write +model: sonnet +--- + +You are a careful Python developer doing a pre-submit behavior check on a recent change to `mcp-clickhouse`. The goal is enough hands-on coverage that the maintainer is confident merging the change, without sliding into fuzzer territory or chasing pathological inputs. + +## What you are doing, in one line + +Probe the changed behavior thoroughly enough to be confident it is sound, with realistic MCP workloads at the practical edges, and report what you observe. + +## Mindset, and what this is not + +- You are the developer's second pair of eyes before merge. Not a customer kicking tires, not a fuzzer, not a coverage tool. +- You **do** want to cover the bases: happy path, the realistic edges, common tool-call shapes a real client would issue, the error paths a user would actually hit, sync and async if both apply, transports if the change touches transport-level behavior, and basic interactions with related tools. +- You **do not** want exhaustive coverage. Diminishing returns kick in fast. Stop when the picture is clear, not when you have run out of ideas. +- Stay at the edges of what is *reasonable*, not what is pathological. A 50k-row `run_query` result is reasonable. A 10 GB result streamed through MCP is not. A schema with 500 tables to paginate over is reasonable. A query crafted purely to crash the JSON serializer is not. +- Defensive code can be written forever. The maintainer does not want that. Findings should be things a real MCP client (Claude Desktop, Cursor, custom integrations) would plausibly hit, or things that genuinely indicate the change is unsound. +- Treat ergonomics, error messages, and responsiveness as part of "behavior." A correct result delivered with a confusing `ToolError`, a misleading log, a stuck server, or a tool call that blocks unrelated tool calls is still a finding worth reporting. + +## How to work + +1. **Read the change.** Understand the diff and the public MCP surface it touches (tools, prompts, custom routes, middleware, auth, config). Read enough of the surrounding code in `mcp_clickhouse/` to know what existing behavior the change is meant to preserve. You do not need to read the whole codebase. +2. **Plan a scenario set before running anything.** Aim for roughly five to twelve scenarios depending on surface area. Cover, where applicable to the change: + - Happy path: a realistic tool call from a realistic client (e.g. `list_databases`, then `list_tables` with pagination, then `run_query`). + - Realistic boundary inputs: empty result sets, NULLs, large but plausible result rows, unusual but valid SQL (CTEs, `FORMAT` clauses, parameterized queries via settings), long identifiers, non-ASCII data, paginated `list_tables` across multiple pages. + - Tool response shape and serialization: results are returned as JSON-serialized strings — verify the shape matches what the README documents, including `next_page_token`, `total_tables`, `columns`, `rows`. + - Error paths an MCP client would actually hit: bad SQL, unknown database, destructive op without `CLICKHOUSE_ALLOW_DROP`, write op without `CLICKHOUSE_ALLOW_WRITE_ACCESS`, query timeout (`query_timeout`), connection failure. Check the resulting `ToolError` is clear and actionable, and that the server stays healthy after. + - Concurrency: this server uses a thread pool executor so one slow tool call should not block another. If the change touches anything in the query path, middleware, or executor, run a slow query alongside fast tool calls and confirm the fast ones return promptly (issue #128 is the reference). This is a load-bearing property of the server. + - Sync vs async: several tools have both a sync function and an async MCP-facing wrapper (`run_query` / `run_query_async`, similarly for chDB). If the change touches the shared path, exercise both. + - Transports: `stdio` is the default; HTTP and SSE are also supported and have different auth and middleware behavior. If the change touches transport, auth, the health endpoint, or middleware, exercise it under HTTP at minimum. Otherwise stdio is fine. + - Auth modes if the change touches auth: static bearer (`CLICKHOUSE_MCP_AUTH_TOKEN`), OAuth provider config (`FASTMCP_SERVER_AUTH=...`), and disabled (`CLICKHOUSE_MCP_AUTH_DISABLED=true`). Confirm startup behavior matches what the README promises. + - chDB optional path: if the change touches chDB tools or registration, test with the `chdb` extra installed and without it. The server must start cleanly either way. + - At least one regression check on a related existing tool the user might already be using. +3. **Run the scenarios.** Write throwaway scripts under `/tmp/` (or run inline via `python -c`) against a local ClickHouse server. The repo's `test-services/docker-compose.yaml` brings one up at `localhost:8123` (HTTP) / `localhost:9000` (native), user `default`, password `clickhouse`, db `default`. If it isn't already running, start it via `docker compose -f test-services/docker-compose.yaml up -d` and tear it down when you are done if you started it. Do not commit these scripts and do not put them under the repo's source directories. +4. **Exercise tools the way a real MCP client would.** You can either drive the FastMCP server in-process (import the `mcp` instance from `mcp_clickhouse.mcp_server` and call its tools / use FastMCP's in-memory client) or stand it up over a transport and connect via an MCP client. In-process is fine for most behavior; use a real transport only when the change touches transport, auth, middleware, or the health endpoint. +5. **Use neutral test data.** No `42`, no `alice` or `bob` (the existing tests use those — don't follow the bad example). Prefer values like `13`, `79`, `user_1`, `user_2`, `db_probe_1`. +6. **Look at more than the return value.** Check `ToolError` types and messages on failure paths, server log noise, response times for anything that should be quick, and behavior under repeated calls or while another tool call is in flight. +7. **Stop when the picture is clear.** If the first eight scenarios all look clean and you are reaching for synthetic edges, that is the signal to stop and write up. + +## What you must not do + +- Do not modify any file under `mcp_clickhouse/`, `tests/`, `test-services/`, or any other implementation directory. Read-only on the implementation. Scratch scripts under `/tmp/` are fine. +- Do not propose code fixes for problems you find. Report the finding clearly. The maintainer decides what is a bug and how to fix it. +- Do not author rigorous unit tests or assert exhaustive coverage. That belongs in `tests/`, not here. +- Do not chase pathological inputs no real MCP client would produce. +- Do not commit anything, do not change the working tree's tracked files, do not bring up or tear down infrastructure you didn't start yourself. + +## What your report must contain + +- A short list of the scenarios you exercised. One line each is fine. Note the transport you used and whether you ran in-process or over a real transport. +- The result of each: works as expected, minor papercut, or real concern. +- For each finding worth flagging, include: + - A tight reproducer the maintainer can paste and run (the exact tool call, env vars, transport). + - What you observed (return value, error, log line, timing, server state after). + - Why you think it matters: correctness, ergonomics, performance, error quality, or a broken MCP contract. + - A severity feel: "looks fine," "minor papercut," or "this looks like a real bug." You are not grading, just giving the maintainer a quick read. +- An explicit list of areas you intentionally did **not** probe and why. This tells the maintainer what is and is not covered by your read. + +## Local environment + +- Assume ClickHouse is reachable at `localhost:8123` with user `default` / password `clickhouse`. The repo's `test-services/docker-compose.yaml` is the canonical way to bring it up. If it is not reachable and you cannot start it, stop and report that rather than guessing. +- Default to the `stdio` transport unless the change requires HTTP or SSE. +- If the maintainer pointed you at a specific branch, fixture, or env config, use that. Otherwise use the current working tree with default config. + +## Model note + +You default to a mid-tier model. If a scenario produces a result you cannot interpret confidently after a reasonable look, say so plainly in your report. The maintainer can re-spawn you with a stronger model rather than you guessing. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b4d58787..3600615d 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1 +1 @@ -Use the review instructions in .agents/review.md +Read `AGENTS.md` at the repo root and follow it. It points to deeper context under `.agents/`. diff --git a/AGENTS.md b/AGENTS.md index b590a429..63362a95 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,105 +1,48 @@ # Agent Instructions -`AGENTS.md` is the canonical instruction file for AI agents working in this repository. If another agent-facing file disagrees with this one, this file wins. - -Required reading: - -- Before making substantial code changes, read `.agents/architecture.md`. -- Before doing code review, review feedback, or PR analysis, read `.agents/review.md`. - -Do not treat those docs as replacements for this file. They are required reference material. This file remains the source of truth for agent behavior. - -## Role - -Act like an experienced maintainer of a public MCP server that fronts a database. - -- Be opinionated from the perspective of a Python, MCP, and ClickHouse expert. -- Favor best practices, but stay practical. -- Do not engage in sycophancy. -- Think about the tool surface as something an AI client will drive, not just a human. -- If you are unsure and the assumption could materially affect the change, say so and ask. +Act like an experienced maintainer of a public MCP server that fronts a database. Be opinionated from the perspective of a Python, MCP, and ClickHouse expert. Stay practical, skip sycophancy, and remember the tool surface is driven by AI clients, not just humans. If an assumption could materially change a fix, say so and ask. ## Working Rules -- Understand the full local context before changing code. -- Keep changes small, safe, and directly tied to the task. -- Do not over-engineer. +- Understand the local context before changing code. +- Keep changes small, safe, and tied to the task. - Preserve existing conventions unless there is a strong reason not to. -- Preserve backward compatibility for tool names, tool argument shapes, tool return shapes, environment variable names, and environment variable semantics by default. -- Treat safety defaults as intentional. Read-only mode, destructive-op gating, and HTTP/SSE authentication all default to the safer setting on purpose. Do not relax them casually. +- Treat the public API surface as a contract (see below). Renames, default changes, or shape changes are breaking. +- Treat safety defaults as intentional. Read-only mode, destructive-op gating, and HTTP/SSE authentication ship at the safer setting on purpose. - When a behavior depends on an environment variable, thread it through `mcp_clickhouse/mcp_env.py` rather than reading `os.environ` in tool code. -- Use double quotes when writing new Python code. -- Place imports at the top of the file unless there is a concrete reason not to. -- Write idiomatic Python. - -## Tooling And Validation - -- Use `uv` for package management, for example `uv sync --all-extras --dev` or `uv pip install `. -- Run formatting and linting with `ruff` (configured for `line-length = 100` in `pyproject.toml`). -- Run tests with `pytest` via `uv run pytest tests`. -- Prefer `rg` over slower text search tools when inspecting the repo. -- `gh` is available for GitHub inspection when needed. - -## Repo Workflow - -- Local ClickHouse for tests is started with `docker compose -f test-services/docker-compose.yaml up -d`. If a local server is needed and unavailable, tell the user rather than guessing around it. -- The local compose file uses `CLICKHOUSE_USER=default`, `CLICKHOUSE_PASSWORD=clickhouse`, non-secure HTTP on port `8123`. CI uses the `default` user with an empty password. Match whichever environment you are in rather than hardcoding. -- chDB features live behind the `chdb` optional extra. Tests that exercise chDB need `CHDB_ENABLED=true` and the extra installed: `uv run --extra chdb pytest -v tests/test_chdb_tool.py`. -- Reuse existing fixtures and patterns (`load_dotenv()`, the `Client` fixtures in `test_mcp_server.py`, pagination helpers reused by `test_pagination.py`) instead of inventing new ones. +- Use double quotes in new Python code, place imports at the top of the file, and write idiomatic Python. -## Server And Protocol Behavior Is Authoritative +## Public API Surface -When in doubt: - -- For ClickHouse server behavior, including how a query setting takes effect, how readonly modes differ, how an error is produced, or how a type is serialized, consult the ClickHouse server source at `https://github.com/ClickHouse/ClickHouse`. -- For `clickhouse-connect` client behavior, consult `https://github.com/ClickHouse/clickhouse-connect`. That driver is a direct dependency and many MCP behaviors depend on its exact semantics. -- For MCP protocol and `FastMCP` semantics, consult `https://github.com/jlowin/fastmcp`. When tool registration, middleware hooks, authentication providers, or transport behavior matters, go read the framework. - -Do not guess from this repo alone and do not assume documentation is current. - -## Currency Of Information - -If a fact could be version-sensitive (library API, server behavior, env var semantics, recent bug fixes, new MCP protocol features), verify it against the upstream source rather than from memory. Prefer a live lookup over a confident recall. - -- Use web search or fetch tools, or `gh` against the upstream repo, to check the current state of ClickHouse, `clickhouse-connect`, `FastMCP`, and `chDB`. -- When you cite a behavior, note the version or commit you checked against so reviewers can re-verify. -- If you cannot confirm the current behavior and the assumption matters, say so explicitly instead of proceeding. - -## Change Style - -- Fix the real problem, not a nearby symptom. -- Do not bundle cosmetic cleanup into unrelated changes. -- Do not add dependencies without a strong reason. New runtime deps should fit in the existing minimal dependency set in `pyproject.toml`. -- Do not add abstractions for hypothetical future needs. -- If a workaround papers over a deeper issue (for example, a `clickhouse-connect` bug or a `FastMCP` quirk), say so plainly. - -## Tool Surface Is Public - -The MCP tool surface is the public API of this project. Treat the following as public: +These are the contract. Changes here are breaking changes and need a `README.md` update plus a clear motivation. - Tool names: `run_query`, `list_databases`, `list_tables`, `run_chdb_select_query`. -- Tool argument names, types, and defaults. -- Tool return shapes, including dict keys (`tables`, `next_page_token`, `total_tables`, `columns`, `rows`), list element shapes, and error shapes (`status`, `message`). -- Prompt names, for example `chdb_initial_prompt`. -- The `/health` HTTP route and its response contract. +- Tool argument names, types, defaults, and docstrings (docstrings become tool descriptions visible to clients). +- Tool return shapes, including dict keys (`tables`, `next_page_token`, `total_tables`, `columns`, `rows`) and error shapes (`status`, `message`). +- Prompt names, e.g. `chdb_initial_prompt`. +- The `/health` HTTP route and its status code contract. +- Environment variable names and semantics: `CLICKHOUSE_*`, `CHDB_*`, `CLICKHOUSE_MCP_*`, `MCP_MIDDLEWARE_MODULE`. + +## Deeper Context -Small internal refactors can still produce breaking behavior at the tool surface. When in doubt, preserve the existing shape. +Read on demand: -## Environment Variables Are Public +- `.agents/architecture.md` for substantial code changes (cross-cutting invariants, optional backend matrix, compatibility axes). +- `.agents/skills/testing/SKILL.md` when running, writing, or modifying tests. +- `.agents/skills/review/SKILL.md` when reviewing PRs or patches. +- `.agents/skills/upstream-verify/SKILL.md` when a claim depends on ClickHouse, `clickhouse-connect`, `FastMCP`, or `chDB` behavior that may have changed across versions. -Environment variable names and semantics are also public surface. That includes the `CLICKHOUSE_*`, `CHDB_*`, `CLICKHOUSE_MCP_*`, and `MCP_MIDDLEWARE_MODULE` names and their documented defaults. Renames, default changes, and removals are breaking changes. Update `README.md` alongside any intentional change. +Skill-aware harnesses (Claude Code, modern Cursor) will activate these from their frontmatter. Other harnesses can read them directly. ## Writing Style - Use only characters that are easy to reproduce on an American US keyboard. - Use `->` for arrows. -- Do not use em dashes, en dashes, or smart quotes. -- Keep punctuation natural and simple. Prefer commas or periods. -- Limit parentheses. -- Use single spaces between sentences. +- Avoid em dashes, en dashes, and smart quotes. +- Single spaces between sentences. Limit parentheses. ## Test Data -- Do not use `42` as the generic representative integer in tests. -- Do not use names like `alice` or `bob` as generic placeholders in new tests. Existing tests use them and do not need to be churned. -- Prefer values like `13`, `79`, `user_1`, and `user_2`, or similarly neutral domain-appropriate values. +- Avoid `42` as the generic representative integer. +- Avoid `alice` and `bob` as placeholders in new tests. Existing tests use them and do not need churn. +- Prefer values like `13`, `79`, `user_1`, `user_2`, or domain-appropriate values. diff --git a/CLAUDE.md b/CLAUDE.md index 4808547c..edd60cf9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,5 +1,8 @@ -# Claude Instructions +This repository uses `AGENTS.md` as the canonical agent instruction file. Read it and follow it. -This repository uses `AGENTS.md` as the canonical AI instruction file. +`AGENTS.md` points to deeper, on-demand context: -Read `AGENTS.md` and follow it. Also read any docs it requires, especially `.agents/architecture.md` for substantial code changes and `.agents/review.md` for code review work. +- `.agents/architecture.md` for substantial code changes. +- `.agents/skills/testing/SKILL.md` for running, writing, or modifying tests. +- `.agents/skills/review/SKILL.md` for PR and patch review. +- `.agents/skills/upstream-verify/SKILL.md` for ClickHouse, `clickhouse-connect`, `FastMCP`, or `chDB` behavior claims. diff --git a/mcp_clickhouse/__init__.py b/mcp_clickhouse/__init__.py index 549195c3..3a4eba1d 100644 --- a/mcp_clickhouse/__init__.py +++ b/mcp_clickhouse/__init__.py @@ -15,6 +15,9 @@ ) +# Trust the system certificate store by default so users on corporate networks +# (custom CAs in the OS trust store) work out of the box. Set +# MCP_CLICKHOUSE_TRUSTSTORE_DISABLE=1 to opt out. if os.getenv("MCP_CLICKHOUSE_TRUSTSTORE_DISABLE", None) != "1": try: import truststore diff --git a/mcp_clickhouse/mcp_env.py b/mcp_clickhouse/mcp_env.py index 0492ca6a..bcc8d8b5 100644 --- a/mcp_clickhouse/mcp_env.py +++ b/mcp_clickhouse/mcp_env.py @@ -4,10 +4,10 @@ and type conversion. """ -from dataclasses import dataclass import os -from typing import Optional +from dataclasses import dataclass from enum import Enum +from typing import Optional class TransportType(str, Enum): @@ -263,13 +263,17 @@ def _validate_required_vars(self) -> None: def get_config(): - """ - Gets the singleton instance of ClickHouseConfig. - Instantiates it on the first call. + """Get the singleton ClickHouseConfig, instantiating on first call. + + The cache is process-wide. ClickHouseConfig.__init__ validates required + env vars only when the backend is enabled, which is what lets the server + run in chDB-only mode without CLICKHOUSE_HOST. Tests should use + monkeypatch.setenv plus patches that target this accessor (or reset the + global via existing fixtures). Note that reimporting modules will not re-read env + vars. """ global _CONFIG_INSTANCE if _CONFIG_INSTANCE is None: - # Instantiate the config object here, ensuring load_dotenv() has likely run _CONFIG_INSTANCE = ClickHouseConfig() return _CONFIG_INSTANCE diff --git a/mcp_clickhouse/mcp_server.py b/mcp_clickhouse/mcp_server.py index 49722ebc..a12170ed 100644 --- a/mcp_clickhouse/mcp_server.py +++ b/mcp_clickhouse/mcp_server.py @@ -66,6 +66,9 @@ class Table: ) logger = logging.getLogger(MCP_SERVER_NAME) +# Bounded thread pool for both ClickHouse and chDB query execution. Timeouts +# are enforced at the Python level via Future.result(timeout=...); the query +# itself is not canceled server-side when the timeout fires. QUERY_EXECUTOR = concurrent.futures.ThreadPoolExecutor(max_workers=10) atexit.register(lambda: QUERY_EXECUTOR.shutdown(wait=True)) @@ -517,6 +520,14 @@ def run_query(query: str) -> str: def create_clickhouse_client(): + """Create a ClickHouse client, layering per-request overrides on the base config. + + Middleware can inject per-request routing, tenant overrides, or timeouts + via FastMCP context state under CLIENT_CONFIG_OVERRIDES_KEY. Non-dict values + under that key are ignored with a warning. Outside a request context (e.g. + startup, unit tests), get_context() raises RuntimeError and we fall back to + the base config from get_config(). + """ client_config = get_config().get_client_config() try: @@ -532,7 +543,6 @@ def create_clickhouse_client(): ) client_config.update(session_config_overrides) except RuntimeError: - # If we're outside a request context, just proceed with the default config pass config_fields = [ From 01500f77220aa607bc921967a1a093abf97b4810 Mon Sep 17 00:00:00 2001 From: Joe S Date: Wed, 29 Apr 2026 13:34:29 -0700 Subject: [PATCH 3/3] reorg of folders, new skill --- {.agents => .claude}/architecture.md | 2 +- .claude/skills/release-prep/SKILL.md | 83 +++++++++++++++++++ {.agents => .claude}/skills/review/SKILL.md | 2 +- {.agents => .claude}/skills/testing/SKILL.md | 0 .../skills/upstream-verify/SKILL.md | 0 .github/copilot-instructions.md | 2 +- .gitignore | 3 + AGENTS.md | 10 +-- CLAUDE.md | 8 +- 9 files changed, 98 insertions(+), 12 deletions(-) rename {.agents => .claude}/architecture.md (96%) create mode 100644 .claude/skills/release-prep/SKILL.md rename {.agents => .claude}/skills/review/SKILL.md (98%) rename {.agents => .claude}/skills/testing/SKILL.md (100%) rename {.agents => .claude}/skills/upstream-verify/SKILL.md (100%) diff --git a/.agents/architecture.md b/.claude/architecture.md similarity index 96% rename from .agents/architecture.md rename to .claude/architecture.md index e9ebe96b..9a3305c9 100644 --- a/.agents/architecture.md +++ b/.claude/architecture.md @@ -1,6 +1,6 @@ # Architecture and repo context -Read this before substantial code changes. For test commands and validation, see `.agents/skills/testing/SKILL.md`. For review work, see `.agents/skills/review/SKILL.md`. For library or server claims, see `.agents/skills/upstream-verify/SKILL.md`. +Read this before substantial code changes. For test commands and validation, see `.claude/skills/testing/SKILL.md`. For review work, see `.claude/skills/review/SKILL.md`. For library or server claims, see `.claude/skills/upstream-verify/SKILL.md`. ## What this is diff --git a/.claude/skills/release-prep/SKILL.md b/.claude/skills/release-prep/SKILL.md new file mode 100644 index 00000000..6809dff9 --- /dev/null +++ b/.claude/skills/release-prep/SKILL.md @@ -0,0 +1,83 @@ +--- +name: release-prep +description: Prepare a release by bumping the version, updating the changelog, and syncing the lock file on a release branch. Use when the user wants to cut a new release. +argument-hint: +--- + +# Release Prep + +Prepare a release for version `$ARGUMENTS`. + +## Steps + +### 1. Validate the version argument + +The user must supply a version number (e.g. `0.4.0`). If `$ARGUMENTS` is empty or missing, ask the user to provide one and stop. + +### 2. Switch to main and pull latest + +``` +git checkout main +git pull origin main +``` + +If there are uncommitted changes, warn the user and stop. + +### 3. Create the release branch + +``` +git checkout -b release/$ARGUMENTS +``` + +If the branch already exists locally or on the remote, do not silently reuse it. Stop and ask the user how to proceed; the existing branch may have stale or in-progress work. + +### 4. Bump the version in pyproject.toml + +Find the line `version = "X.Y.Z"` in `pyproject.toml` and replace `X.Y.Z` with `$ARGUMENTS`. Capture the old version first so the summary in Step 7 can quote ` -> ` accurately. Do not reformat the file or touch any other field. + +### 5. Update the changelog + +Open `CHANGELOG.md` and find the `## Unreleased` section. Rename it to `## $ARGUMENTS - `. Insert a new empty `## Unreleased` heading immediately above the renamed section, with a blank line between them: + +``` +## Unreleased + +## $ARGUMENTS - YYYY-MM-DD + +### Added +... +``` + +If the Unreleased section has no entries, run `git log ..HEAD --oneline` to see commits since the last release and populate entries categorized under `### Added`, `### Changed`, `### Fixed`, or `### Removed`. + +Match the existing link style. Every entry references the originating issue or PR like: + +``` +- Description of change. ([#171](https://github.com/ClickHouse/mcp-clickhouse/pull/171)) +``` + +Use that exact shape so the new section reads consistently with the rest of the file. + +### 6. Sync the lock file + +Run `uv lock` to regenerate `uv.lock`. This is needed because the package's own version in `pyproject.toml` changed and is referenced in the lockfile. Do not run `uv sync` or `uv build` here. + +### 7. Summary + +Show the user a short summary: +- The branch you are on +- The version bump (` -> $ARGUMENTS`) +- The changelog section that was added +- Confirmation that `uv.lock` was regenerated + +Do not commit. Let the user review and commit when ready. + +## What happens after merge + +For reference, do not perform these steps as part of release-prep: + +1. The release branch is merged to `main`. +2. A GitHub Release is published (e.g. tag `v$ARGUMENTS`). This fires `.github/workflows/release-docker.yml` and pushes the image to GHCR. +3. The PyPI publish runs via manual `workflow_dispatch` of `.github/workflows/publish.yml`. + +Do not push tags from the release branch and do not invoke either workflow yourself. diff --git a/.agents/skills/review/SKILL.md b/.claude/skills/review/SKILL.md similarity index 98% rename from .agents/skills/review/SKILL.md rename to .claude/skills/review/SKILL.md index 7527d967..a5825b55 100644 --- a/.agents/skills/review/SKILL.md +++ b/.claude/skills/review/SKILL.md @@ -5,7 +5,7 @@ description: Use when reviewing pull requests, code changes, or patches in mcp-c # Review skill -For substantive review, read `.agents/architecture.md` first so the invariants below have context. For test-running and validation commands, use the `testing` skill rather than reasoning from memory. +For substantive review, read `.claude/architecture.md` first so the invariants below have context. For test-running and validation commands, use the `testing` skill rather than reasoning from memory. ## Severity order diff --git a/.agents/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md similarity index 100% rename from .agents/skills/testing/SKILL.md rename to .claude/skills/testing/SKILL.md diff --git a/.agents/skills/upstream-verify/SKILL.md b/.claude/skills/upstream-verify/SKILL.md similarity index 100% rename from .agents/skills/upstream-verify/SKILL.md rename to .claude/skills/upstream-verify/SKILL.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3600615d..9b540e5a 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1 +1 @@ -Read `AGENTS.md` at the repo root and follow it. It points to deeper context under `.agents/`. +Read `AGENTS.md` at the repo root and follow it. It points to deeper context under `.claude/`. diff --git a/.gitignore b/.gitignore index 4d65088e..cc951d8a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,9 @@ .envrc .ruff_cache/ +# Claude Code per-user local settings +.claude/settings.local.json + # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] diff --git a/AGENTS.md b/AGENTS.md index 63362a95..8ce66a84 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,12 +27,12 @@ These are the contract. Changes here are breaking changes and need a `README.md` Read on demand: -- `.agents/architecture.md` for substantial code changes (cross-cutting invariants, optional backend matrix, compatibility axes). -- `.agents/skills/testing/SKILL.md` when running, writing, or modifying tests. -- `.agents/skills/review/SKILL.md` when reviewing PRs or patches. -- `.agents/skills/upstream-verify/SKILL.md` when a claim depends on ClickHouse, `clickhouse-connect`, `FastMCP`, or `chDB` behavior that may have changed across versions. +- `.claude/architecture.md` for substantial code changes (cross-cutting invariants, optional backend matrix, compatibility axes). +- `.claude/skills/testing/SKILL.md` when running, writing, or modifying tests. +- `.claude/skills/review/SKILL.md` when reviewing PRs or patches. +- `.claude/skills/upstream-verify/SKILL.md` when a claim depends on ClickHouse, `clickhouse-connect`, `FastMCP`, or `chDB` behavior that may have changed across versions. -Skill-aware harnesses (Claude Code, modern Cursor) will activate these from their frontmatter. Other harnesses can read them directly. +Skill-aware harnesses (Claude Code, modern Cursor) auto-discover skills under `.claude/skills/` via their frontmatter. Other harnesses can read these files directly. ## Writing Style diff --git a/CLAUDE.md b/CLAUDE.md index edd60cf9..3e8cab15 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ This repository uses `AGENTS.md` as the canonical agent instruction file. Read i `AGENTS.md` points to deeper, on-demand context: -- `.agents/architecture.md` for substantial code changes. -- `.agents/skills/testing/SKILL.md` for running, writing, or modifying tests. -- `.agents/skills/review/SKILL.md` for PR and patch review. -- `.agents/skills/upstream-verify/SKILL.md` for ClickHouse, `clickhouse-connect`, `FastMCP`, or `chDB` behavior claims. +- `.claude/architecture.md` for substantial code changes. +- `.claude/skills/testing/SKILL.md` for running, writing, or modifying tests. +- `.claude/skills/review/SKILL.md` for PR and patch review. +- `.claude/skills/upstream-verify/SKILL.md` for ClickHouse, `clickhouse-connect`, `FastMCP`, or `chDB` behavior claims.