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/.claude/architecture.md b/.claude/architecture.md new file mode 100644 index 00000000..9a3305c9 --- /dev/null +++ b/.claude/architecture.md @@ -0,0 +1,80 @@ +# Architecture and repo context + +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 + +`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. + +## Layout + +```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 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 +``` + +## Cross-cutting invariants + +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. + +### Layered safety defaults + +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 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. + +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. + +### Two independently optional backends + +ClickHouse and chDB can each be enabled or disabled. All four combinations must keep working: + +- `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 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. + +### Tool surface and env vars are public + +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 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. 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 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. + +## Performance and resource notes + +The server is not a hot path, but a few areas matter: + +- `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. + +## Compatibility axes + +- 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`. +- 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. + +## When code and this doc disagree + +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/.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/.claude/skills/review/SKILL.md b/.claude/skills/review/SKILL.md new file mode 100644 index 00000000..a5825b55 --- /dev/null +++ b/.claude/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 `.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 + +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/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md new file mode 100644 index 00000000..8d4d81c4 --- /dev/null +++ b/.claude/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/.claude/skills/upstream-verify/SKILL.md b/.claude/skills/upstream-verify/SKILL.md new file mode 100644 index 00000000..9dc8dc27 --- /dev/null +++ b/.claude/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/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..9b540e5a --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1 @@ +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 new file mode 100644 index 00000000..8ce66a84 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,48 @@ +# Agent Instructions + +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 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. +- 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 in new Python code, place imports at the top of the file, and write idiomatic Python. + +## Public API Surface + +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, 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 + +Read on demand: + +- `.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) auto-discover skills under `.claude/skills/` via their frontmatter. Other harnesses can read these files directly. + +## Writing Style + +- Use only characters that are easy to reproduce on an American US keyboard. +- Use `->` for arrows. +- Avoid em dashes, en dashes, and smart quotes. +- Single spaces between sentences. Limit parentheses. + +## Test Data + +- 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 new file mode 100644 index 00000000..3e8cab15 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,8 @@ +This repository uses `AGENTS.md` as the canonical agent instruction file. Read it and follow it. + +`AGENTS.md` points to deeper, on-demand context: + +- `.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. 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 = [