Skip to content

Commit 19b8977

Browse files
authored
Fastmcp3 (#329)
* feat: add ToolAnnotations to all tool registrations (#317) (#327) Attach MCP ToolAnnotations hints to every registered tool based on name prefix, so LLM clients can distinguish read-only queries from destructive operations without relying on description text. - Add _PREFIX_ANNOTATIONS and _TOOL_ANNOTATIONS module-level constants in app.py, plus _annotations_for() helper that checks per-tool overrides first then falls back to prefix matching. - Update all 6 mcp.tool() registration sites (handle_* tools, tdml_* dynamic tools, YAML cube/query tools, registry tools) to pass annotations=_annotations_for(name). - Annotation map: base_, dba_, sec_, rag_, qlty_, graph_, sql_, plot_, tdvs_ -> readOnlyHint=True/idempotentHint=True; bar_ -> destructiveHint=True; tdml_ -> idempotentHint=True; tdvs_grant_user/tdvs_revoke_user override the read-only tdvs_ default with destructiveHint=True. - Add tests/verify_tool_annotations.py: standalone script that calls list_tools via MCP client and asserts expected hints — runs without a live Teradata connection. - Document the prefix-based annotation system in docs/developer_guide/HOW_TO_ADD_YOUR_FUNCTION.md. * feat: add PingMiddleware for HTTP keep-alive and close stdio/HTTP test gap (#318) - Add PingMiddleware to streamable-http and sse transports to prevent idle-timeout disconnections through load balancers and reverse proxies. Interval is configurable via MCP_PING_INTERVAL env var (default 30s). - Add tests/smoke_http.py: starts the server in streamable-http mode, connects via MCP HTTP client, and calls list_tools. No DATABASE_URI required — catches transport-level startup errors that the stdio suite cannot reach. - Extend run_mcp_tests.py with --transport streamable-http to run the full integration test suite over HTTP against a live database. - Update CI to add smoke-http (always runs), test-stdio, and test-http jobs replacing the single integration test job. - Consolidate test cases: migrate missing cases from test_cases.json into core_test_cases.json and new registry_test_cases.json, then remove the now-redundant test_cases.json. - Update CONFIGURATION.md, DEVELOPER_GUIDE.md, CONTRIBUTING.md, and CI.md to document MCP_PING_INTERVAL and the full pre-push checklist. * feat: use ctx.transport in middleware instead of constructor-injected string (#319) Replace the constructor-injected transport parameter on RequestContextMiddleware with a live read of context.fastmcp_context.transport at request time. Adds unit tests for all three transport-path branches (stdio, http, None fallback) folded into the smoke-http CI job. * feat: add ErrorHandlingMiddleware and mask_error_details to prevent internal detail leakage (#320) Replace ad-hoc format_error_response() calls with raise ToolError() throughout app.py, add ErrorHandlingMiddleware and mask_error_details=True to FastMCP, and remove the now-unused format_error_response helper. Adds expect_error test case support and a negative-path test to verify masked error behaviour. * feat: replace get_tdconn() closure with FastMCP lifespan for guaranteed engine.dispose() (#325) Replace the nonlocal-mutating get_tdconn() closure with a FastMCP v3 teradata_lifespan async context manager. TDConn is created once at server startup and engine.dispose() is guaranteed in the finally block on shutdown, eliminating the connection pool leak on Ctrl-C/SIGTERM. - Add _ConnState class to hold tdconn and fs_config (replaces nonlocal vars) - Add teradata_lifespan(server) context manager passed as lifespan= to FastMCP - Move teradataml context, EFS/FeatureStoreConfig, TDVS setup, chat DB validation, tdml_* tool registration, and registry initial load into lifespan - execute_db_tool reads _state.tdconn directly; raises ToolError if no engine - Middleware tdconn_supplier changed to lambda: _state.tdconn - BAR and chat config-file validation remain in factory (no DB needed) - Remove recreate flag from connection management — lifespan owns the lifecycle - Update CLAUDE.md and DEVELOPER_GUIDE.md for the new pattern - Add two middleware tests for the lambda supplier pattern
1 parent 86e5c52 commit 19b8977

20 files changed

Lines changed: 964 additions & 865 deletions

.github/CI.md

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ The CI workflow (`.github/workflows/ci.yml`) runs on every push to `main` and on
66

77
### Jobs
88

9-
| Job | What it does |
10-
|-----|-------------|
11-
| **Lint** | Runs `ruff check` and `ruff format --check` against `src/` |
12-
| **Type Check** | Runs `mypy` against `src/` (installs the `dev` extra for type stubs) |
13-
| **Integration Tests** | Runs the test suite against a live Teradata database |
9+
| Job | What it does | Database required |
10+
|-----|-------------|:-----------------:|
11+
| **Lint** | Runs `ruff check` and `ruff format --check` against `src/` | No |
12+
| **Type Check** | Runs `mypy` against `src/` (installs the `dev` extra for type stubs) | No |
13+
| **HTTP Transport Smoke Test** | Runs middleware unit tests (transport-path branching, no database) then starts the server in `streamable-http` mode, connects via the MCP HTTP client, calls `list_tools`, and shuts down. Catches startup-time errors in HTTP-specific code paths (middleware registration, import errors) that the stdio suite cannot reach. | No |
14+
| **Integration Tests (stdio)** | Runs the full test suite over stdio against a live Teradata database | Yes |
15+
| **Integration Tests (streamable-http)** | Runs the full test suite over HTTP against a live Teradata database | Yes |
1416

1517
All jobs use `uv sync --frozen` to ensure the lock file is up to date — if `uv.lock` is stale relative to `pyproject.toml`, the job will fail.
1618

@@ -25,9 +27,19 @@ uv run ruff format --check src/
2527
uv sync --extra dev
2628
uv run mypy src/
2729

28-
# Integration tests (requires a live Teradata connection)
30+
# Middleware unit tests (no database required)
31+
uv run python tests/middleware_transport_tests.py
32+
33+
# HTTP transport smoke test (no database required)
34+
uv run python tests/smoke_http.py --verbose
35+
36+
# Integration tests — stdio (requires a live Teradata connection)
2937
export DATABASE_URI="teradata://user:pass@host:1025/database"
3038
uv run python tests/run_mcp_tests.py "uv run teradata-mcp-server"
39+
40+
# Integration tests — streamable-http (requires a live Teradata connection)
41+
export DATABASE_URI="teradata://user:pass@host:1025/database"
42+
uv run python tests/run_mcp_tests.py "uv run teradata-mcp-server" --transport streamable-http
3143
```
3244

3345
### Configuring the `DATABASE_URI` Secret

.github/workflows/ci.yml

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,25 @@ jobs:
4040
- run: uv sync --frozen --extra dev
4141
- run: uv run mypy src/
4242

43-
test:
44-
name: Integration Tests
43+
smoke-http:
44+
name: HTTP Transport Smoke Test
45+
runs-on: ubuntu-latest
46+
steps:
47+
- uses: actions/checkout@v4
48+
- uses: astral-sh/setup-uv@v4
49+
with:
50+
enable-cache: true
51+
- uses: actions/setup-python@v5
52+
with:
53+
python-version: "3.11"
54+
- run: uv sync --frozen
55+
- name: Run middleware unit tests (no database required)
56+
run: uv run python tests/middleware_transport_tests.py
57+
- name: Run HTTP smoke test (no database required)
58+
run: uv run python tests/smoke_http.py --verbose
59+
60+
test-stdio:
61+
name: Integration Tests (stdio)
4562
runs-on: ubuntu-latest
4663
if: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }}
4764
steps:
@@ -53,7 +70,7 @@ jobs:
5370
with:
5471
python-version: "3.11"
5572
- run: uv sync --frozen
56-
- name: Run integration tests
73+
- name: Run stdio integration tests
5774
if: ${{ env.DATABASE_URI != '' }}
5875
env:
5976
DATABASE_URI: ${{ secrets.DATABASE_URI }}
@@ -62,4 +79,28 @@ jobs:
6279
if: ${{ env.DATABASE_URI == '' }}
6380
env:
6481
DATABASE_URI: ${{ secrets.DATABASE_URI }}
65-
run: echo "::warning::DATABASE_URI secret is not configured — integration tests were skipped. See .github/README.md for setup instructions."
82+
run: echo "::warning::DATABASE_URI secret is not configured — stdio integration tests were skipped. See .github/CI.md for setup instructions."
83+
84+
test-http:
85+
name: Integration Tests (streamable-http)
86+
runs-on: ubuntu-latest
87+
if: ${{ github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository }}
88+
steps:
89+
- uses: actions/checkout@v4
90+
- uses: astral-sh/setup-uv@v4
91+
with:
92+
enable-cache: true
93+
- uses: actions/setup-python@v5
94+
with:
95+
python-version: "3.11"
96+
- run: uv sync --frozen
97+
- name: Run HTTP integration tests
98+
if: ${{ env.DATABASE_URI != '' }}
99+
env:
100+
DATABASE_URI: ${{ secrets.DATABASE_URI }}
101+
run: uv run python tests/run_mcp_tests.py "uv run teradata-mcp-server" --transport streamable-http
102+
- name: Warn if DATABASE_URI not configured
103+
if: ${{ env.DATABASE_URI == '' }}
104+
env:
105+
DATABASE_URI: ${{ secrets.DATABASE_URI }}
106+
run: echo "::warning::DATABASE_URI secret is not configured — HTTP integration tests were skipped. See .github/CI.md for setup instructions."

CLAUDE.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ docker compose up teradata-mcp-server
4444
### Request Flow
4545

4646
1. **`server.py`** — CLI argument parsing, calls `create_mcp_app()`
47-
2. **`app.py`** — FastMCP app factory. Creates the MCP instance, registers tools/prompts/resources based on profile, configures middleware
47+
2. **`app.py`** — FastMCP app factory. Creates the MCP instance, registers tools/prompts/resources based on profile, configures middleware. A `teradata_lifespan` async context manager (passed to `FastMCP(lifespan=...)`) owns the `TDConn` pool lifecycle: creates the pool at server startup and calls `engine.dispose()` in its `finally` block on shutdown.
4848
3. **`middleware.py`**`RequestContextMiddleware` extracts per-request headers, auth, and session info. Sets Teradata QueryBand for tracing
4949
4. **Tool handlers** — Plain sync functions (`handle_*`) in `tools/` subdirectories. Wrapped to async via `asyncio.to_thread`
5050

@@ -73,7 +73,7 @@ The ~89 `tdml_*` tools (e.g., `tdml_KMeans`, `tdml_XGBoost`) are registered dyna
7373
- **`tools/constants.py`**`TD_ANALYTIC_FUNCS`: a `dict[str, str]` mapping teradataml function name → curated one-line summary. This is the authoritative list of which functions to register. To add a new function, add one entry here.
7474
- **`tools/utils/__init__.py`**`build_tdml_tool_docstring(summary, func_metadata, partition_order_cols)`: builds the compact MCP tool description at registration time by reading parameter names, descriptions, and types from the live teradataml JSON store.
7575

76-
Tools are only registered when `enable_analytic_functions` is true, teradataml is installed, and a database connection is available. Functions missing from the connected system are skipped with a warning.
76+
Tools are registered inside `teradata_lifespan` at server startup (after the DB connection and teradataml context are confirmed). This means `tdml_*` tools become available once the lifespan completes, not at factory time. Functions missing from the connected system are skipped with a warning.
7777

7878
### Configuration System
7979

@@ -87,6 +87,8 @@ Settings dataclass in `config/__init__.py` merges CLI args, environment variable
8787

8888
`TDConn` class in `tools/td_connect.py` manages SQLAlchemy engine creation for Teradata. Supports connection pooling (`TD_POOL_SIZE`, `TD_MAX_OVERFLOW`, `TD_POOL_TIMEOUT`), auth modes (TD2, LDAP via `LOGMECH`), and rate-limited authentication.
8989

90+
The `TDConn` instance is created inside the `teradata_lifespan` context manager in `app.py` and stored in a `_ConnState` holder (`_state.tdconn`). This guarantees `engine.dispose()` runs on server shutdown. If `DATABASE_URI` is not set, `TDConn` sets `engine = None` and the server starts without a database (tools will raise at invocation time).
91+
9092
Tool handlers receive either a SQLAlchemy `Connection` or raw `TeradataConnection` as their first parameter — the wrapper in `app.py` handles injection.
9193

9294
`base_readQuery` caps result rows to prevent LLM token overflow: default 1000 rows, hard ceiling 50000. Configurable via `DEFAULT_ROW_LIMIT` and `MAX_ROW_LIMIT` env vars. When truncated, response metadata includes `truncated: true`; callers can pass a higher `row_limit` or use `persist=true` to bypass the cap.

docs/developer_guide/CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Make sure you have setup your environment based on the Developer Guide in this r
77
## Development Guidelines
88
- Always engage on the discussion board or create an issue before creating a PR.
99
- All PRs must have at least one issue associated.
10-
- Run testing before PR, and copy/paste the test report status in the PR. You can simply run the mandatory test tools with `python tests/run_mcp_tests.py "uv run teradata-mcp-server"`. For more information see [our testing guide](/tests/README.md)
10+
- Run the full pre-push checklist before opening a PR, and copy/paste the test report output in the PR: lint (`uv run ruff check src/`), type check (`uv run mypy src/`), HTTP smoke test (`uv run python tests/smoke_http.py`), and integration tests over both stdio and HTTP (`uv run python tests/run_mcp_tests.py "uv run teradata-mcp-server"` and `... --transport streamable-http`). See the [Developer Guide](./DEVELOPER_GUIDE.md) for the full ordered checklist.
1111
- Create a new test case if you add a new tool. For more information see [our testing guide](/tests/README.md)
1212
- All code must be reviewed via a pull request. Before anything can be merged, it must be reviewed by at least 2 others. [Contributing to a project step by step instuctions](https://docs.github.com/en/get-started/exploring-projects-on-github/contributing-to-a-project)
1313
- Squash commits into a single commit for your PR. We want to keep a clean git history.

docs/developer_guide/DEVELOPER_GUIDE.md

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,33 @@ npx modelcontextprotocol/inspector uv run teradata-mcp-server
3838

3939
### 5) Run tests
4040

41-
Always run tests before submitting a PR:
41+
Always run the full pre-push checklist before submitting a PR. Run steps in order — the fast checks come first so you fail early.
4242

4343
```bash
44-
# core tests
45-
python tests/run_mcp_tests.py "uv run teradata-mcp-server"
44+
# 1. Lint (no database required)
45+
uv run ruff check src/
46+
uv run ruff format --check src/
47+
48+
# 2. Type check (no database required)
49+
uv sync --extra dev
50+
uv run mypy src/
51+
52+
# 3. HTTP transport smoke test (no database required)
53+
# Catches startup-time errors in HTTP-specific code paths (middleware,
54+
# imports, constructor errors) that the stdio suite cannot reach.
55+
uv run python tests/smoke_http.py --verbose
56+
57+
# 4. Integration tests — stdio (requires DATABASE_URI)
58+
export DATABASE_URI="teradata://user:pass@host:1025/database"
59+
uv run python tests/run_mcp_tests.py "uv run teradata-mcp-server"
60+
61+
# 5. Integration tests — streamable-http (requires DATABASE_URI)
62+
uv run python tests/run_mcp_tests.py "uv run teradata-mcp-server" --transport streamable-http
4663
```
47-
Most users use the MCP server with Claude, stdio, so always test Claude's behaviour with your code or build before pushing it.
64+
65+
Steps 1–3 have no database dependency and are quick — run them on every change. Steps 4–5 require VPN and credentials; run them when you have changed tool handlers, middleware, or connection logic.
66+
67+
Most users use the MCP server with Claude over stdio, so always test Claude's behaviour with your code or build before pushing it.
4868

4969
Example configurations:
5070
```json
@@ -106,7 +126,7 @@ The directory structure will follow the following conventions
106126
- `config.py`: `Settings` dataclass and `settings_from_env()` for centralized configuration (single source of truth; precedence is CLI > env > defaults).
107127
- `utils.py` (logging): structured logging setup (stdio‑safe) and JSON formatter.
108128
- `middleware.py`: shared `RequestContextMiddleware` that extracts per-request context; has a stdio fast-path (no headers/auth) and a full HTTP/SSE path that can enforce auth and cache.
109-
- MCP adapter (inlined in `app.py`): internal `execute_db_tool` (DB connection injection, QueryBand, error handling) and `make_tool_wrapper` (auto MCP wrapper for `handle_*` functions).
129+
- MCP adapter (inlined in `app.py`): internal `execute_db_tool` (DB connection injection, QueryBand, raises `ToolError` on failure) and `make_tool_wrapper` (auto MCP wrapper for `handle_*` functions). `ErrorHandlingMiddleware` with `mask_error_details=True` ensures raw SQL errors and stack traces are never forwarded to LLM clients.
110130
- `tools/utils/queryband.py`: pure helpers to build Teradata QueryBand strings from request context (protocol-agnostic).
111131
- `utils.py`: configuration helpers for profiles and YAML object loading.
112132
- `testing/`: testing framework and utilities.
@@ -373,17 +393,17 @@ This section explains how the pieces fit together at runtime.
373393
- The wrapper delegates execution to `execute_db_tool` which:
374394
- Injects a DB connection (SQLAlchemy `Connection` preferred)
375395
- Sets QueryBand based on request context (`tools/utils/queryband.py`)
376-
- Dynamically registers `tdml_*` analytic function tools when teradataml is installed and a database connection is available (see below).
396+
- Dynamically registers `tdml_*` analytic function tools via the `teradata_lifespan` context manager, after the database connection and teradataml context are confirmed (see below).
377397

378398
### Dynamic teradataml Analytic Function Registration
379399

380-
The ~89 `tdml_*` tools (e.g., `tdml_KMeans`, `tdml_XGBoost`) are not defined as `handle_*` functions. Instead, `app.py` generates and registers them at startup when `enable_analytic_functions` is true:
400+
The ~89 `tdml_*` tools (e.g., `tdml_KMeans`, `tdml_XGBoost`) are not defined as `handle_*` functions. Instead, `app.py` generates and registers them inside the `teradata_lifespan` async context manager at server startup, after the teradataml context is established:
381401

382402
1. **`tools/constants.py`** — `TD_ANALYTIC_FUNCS` is a `dict[str, str]` mapping each teradataml function name to a curated one-line summary (e.g., `"KMeans": "Groups observations into k clusters..."`). This dict is the authoritative list of which functions to register.
383403
384404
2. **`tools/utils/__init__.py`** — `build_tdml_tool_docstring(summary, func_metadata, partition_order_cols)` builds the compact MCP tool description at registration time. It reads parameter names, descriptions, Required/Optional, and types directly from `func_metadata.arguments` (teradataml's live JSON store, populated from the database), combining them with the curated summary.
385405

386-
3. **`app.py`** — Iterates `TD_ANALYTIC_FUNCS.items()`, queries the live JSON store for each function's metadata, generates a Python function string via `exec()`, and registers it with `mcp.tool()`. If a function from the dict is not present in the connected database's function list, it is skipped with a warning.
406+
3. **`app.py` (`teradata_lifespan`)** — After confirming the teradataml context is available, iterates `TD_ANALYTIC_FUNCS.items()`, queries the live JSON store for each function's metadata, generates a Python function string via `exec()`, and registers it with `server.tool()`. If a function from the dict is not present in the connected database's function list, it is skipped with a warning. Registration happens once at startup before any client connections are accepted.
387407

388408
**To add a new analytic function:** add one entry to `TD_ANALYTIC_FUNCS` in `tools/constants.py` with a concise one-line description. No other code changes are needed.
389409

docs/developer_guide/HOW_TO_ADD_YOUR_FUNCTION.md

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def handle_fs_myFunctionName(
6565

6666
except Exception as e:
6767
logger.error(f"Error in handle_fs_myFunctionName: {e}")
68-
return create_response({"error": str(e)}, {"tool_name": "fs_myFunctionName"})
68+
raise
6969
```
7070

7171
---
@@ -84,6 +84,26 @@ fs:
8484
```
8585

8686

87+
---
88+
89+
### 🏷️ Tool Annotations (automatic)
90+
91+
Every registered tool receives an MCP `ToolAnnotations` object based on its name prefix. These hints tell LLM clients whether a tool is safe to run silently (`readOnlyHint`) or requires a confirmation prompt (`destructiveHint`).
92+
93+
| Prefix | Hint applied |
94+
|--------|-------------|
95+
| `base_`, `dba_`, `sec_`, `rag_`, `qlty_`, `graph_`, `sql_`, `plot_`, `tdvs_` | `readOnlyHint=True, idempotentHint=True` |
96+
| `bar_` | `readOnlyHint=False, destructiveHint=True` |
97+
| `tdml_` | `readOnlyHint=False, idempotentHint=True` |
98+
| `chat_`, `fs_`, unknown prefixes | No annotation (MCP default) |
99+
100+
Per-tool overrides exist for `tdvs_grant_user` and `tdvs_revoke_user` (marked destructive despite the read-only `tdvs_` prefix).
101+
102+
When adding a new tool:
103+
- **Existing prefix** — no action needed; the correct annotation is inherited automatically.
104+
- **New prefix** — add an entry to `_PREFIX_ANNOTATIONS` in `app.py`.
105+
- **Exception within a read-only prefix** (e.g., a future `dba_dropTable`) — add a per-tool entry to `_TOOL_ANNOTATIONS` in `app.py`.
106+
87107
---
88108

89109
### 🛠️ What the server does for you

docs/server_guide/CONFIGURATION.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export DATABASE_URI="teradata://username:password@host:1025/database"
2828
export MCP_TRANSPORT="stdio" # or "streamable-http"
2929
export MCP_HOST="localhost" # for HTTP transport
3030
export MCP_PORT="8001" # for HTTP transport
31+
export MCP_PING_INTERVAL="30" # keep-alive ping interval (seconds) for streamable-http and sse transports
3132
export PROFILE="all" # tool profile to load
3233
export LOGGING_LEVEL="WARNING" # DEBUG, INFO, WARNING, ERROR
3334

@@ -293,6 +294,16 @@ For specialized streaming applications:
293294
teradata-mcp-server --mcp_transport sse --mcp_port 8001
294295
```
295296

297+
### Keep-Alive (HTTP/SSE transports)
298+
299+
Load balancers and reverse proxies (nginx, AWS ALB) close idle connections after a fixed timeout. For long-running Teradata queries over `streamable-http` or `sse`, the server sends periodic ping messages to keep the connection alive.
300+
301+
```bash
302+
export MCP_PING_INTERVAL="30" # seconds between keep-alive pings (default: 30)
303+
```
304+
305+
This has no effect on `stdio` transport.
306+
296307
## 🔒 Authentication Configuration
297308

298309
### No Authentication (Default)

env

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ MCP_TRANSPORT=streamable-http #stdio, sse, streamable-http
99
MCP_HOST=0.0.0.0
1010
MCP_PORT=8001
1111
MCP_PATH=/mcp/
12+
MCP_PING_INTERVAL=30 # keep-alive ping interval (seconds) for streamable-http and sse transports
1213

1314
# ----- Enterprise Vector Store ----------
1415
TD_BASE_URL=https://host/api/accounts/40c83ff23b2e #Your UES_URI, strip off the trailing /open-analytics

0 commit comments

Comments
 (0)