diff --git a/README.md b/README.md index c06b8769..4daae79a 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,48 @@ Both formats are binary by default; `--ascii` writes text you can read with `head`. Every mesh file records what it was made from, and stderr gets a one-line summary of the run. +## Connect an agent + +An AI agent (Claude Desktop, Codex, or any other client of the Model +Context Protocol, MCP) can drive rasputin through a small server built in. +Install the `mcp` extra (the `dev` extra includes it): + +```sh +pip install "rasputin[mcp]" +``` + +`rasputin mcp` then serves the commands over stdin and stdout. Point the +client at the `rasputin` of the environment you installed into, or at +`python -m tin_engine mcp` with that environment's `python`. Claude Desktop +takes this in `claude_desktop_config.json` (on macOS under +`~/Library/Application Support/Claude/`): + +```json +{"mcpServers": {"rasputin": {"command": "/path/to/venv/bin/rasputin", "args": ["mcp"]}}} +``` + +Codex takes the same two fields in `~/.codex/config.toml`: + +```toml +[mcp_servers.rasputin] +command = "/path/to/venv/bin/rasputin" +args = ["mcp"] +``` + +The agent sees one tool per rasputin command (`mesh`, `catchment`, `fetch`, +`fetch_stations`, ...; the names use underscores), each with the command's +options as its input schema, help text included, so an option added to the +CLI is a new tool argument with no further work. A call returns the exit code, +stdout and stderr, each output file as a `file://` link and, for `mesh`, the +run record and the `--stats` report as data; the mesh itself never travels +through the agent. A refused command comes back as an error carrying the +CLI's own `Error:` words. During a long run the server reports every line the +command prints to stderr and a heartbeat every 5 s while it is silent; cancelling the +call kills the run. One call runs at a time unless `--jobs N` says more. One +caveat: a `mesh` cancelled while writing a `.vtk` or `.ply` can leave a +truncated file under the asked name, so rerun before reading it; a `.gpkg` +goes through a `.part` file and a rename, so it cannot. + ## How it is built - **A C++20 core** (`include/terrain/`, `src/`): exact geometric predicates, diff --git a/ROADMAP.md b/ROADMAP.md index e14a3804..1f27f21f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -57,6 +57,7 @@ increment that most needs a picture to check against | 31 | Binary output by default: `rasputin mesh` writes binary `.vtk` (and `.ply`) unless `--ascii` is given, because the text writer formats one number at a time and costs minutes at basin scale (the performance review's finding F6 and question 3, PR #208). Ola, 2026-10-07: "change 3 to default binary". The flag pair `--binary/--ascii` stays; the help says binary is the default; README updated. Python only; no `@perf` run (not refine or mesh code) | **merged (#212, 2026-10-07)** (designed 2026-10-07; red `e3fe391a`/`9008811c`, green `29bc5ad3`; 4 net production lines against about 1, the overrun is `ruff format` wrapping the option); Ola ruled `.ply` binary too, as built; next the push | `docs/increments/31-binary-default.md` | | 32 | Land-cover borders simplified within a band, each class keeping its area exactly (Ola, 2026-10-08: "we need to implement is a CORINE simplification, similar to what we do on the outline of the auto-catchments"): the land-cover coverage cut into borders between junctions, each border reduced by the area-preserving segment collapse of the catchment outline (`reduce_ring`), with junctions and the domain outline fixed, a crossing test and a side test across all borders, and a band no border moves past (two-sided, proved by anchors on the source border). `--features-tolerance` becomes that band, default 50 m (half CORINE's positional accuracy), replacing GEOS's coverage simplifier; start angle 15° when land cover is simplified (band above 0), 25° otherwise. Prototype on the German fused catchment: 5.7 times fewer triangles at 50 m vertical tolerance | **merged (#222, 2026-10-11)** (green `82c0a7d4`, 519 net production lines, one PR; mutation round `30f939b6`, one mutant recorded as not killed); Ola ruled all four questions (2026-10-08); code review round 1 asked for changes (comments, docstrings and `project_structure.md` the change made false), fixed by `4d4f0411` and the docs commit after it; code review rounds 1-3 done (round 3 approved); `@perf`'s short timing check found the runs about 70 % slower (the outline rule on borders clipped to the catchment) and the worst angle far lower (the simplifier placing points millimetres from junctions); fix built (section 15: outline rule first, a 1 m clearance in the simplifier; green `05c8f73a`, 573 counted lines in all; mutation round M7 to M13 killed); its quick check `b192a2c6`: Numedalslagen −11 %, Lagan −1.3 %, outline rule and moved area as before, triangles −42 %; questions 5 and 6 decided on default while Ola was away, reversible; fix code review round 1 asked for prose fixes and the test file's red-step staging removed; fix code review round 3 approved; next the push on Ola's yes | `docs/increments/32-landcover-simplify.md` | | 36 | GeoPackage output, so QGIS opens a mesh as it opens any vector layer: `rasputin mesh --out X.gpkg`, chosen by the suffix of `--out` as `.vtk` and `.ply` are (Ola, 2026-10-05: "Is the vtk_to_phkg.py the way to go, or as a direct output from rasputin? I think the latter, and it should go into the roadmap and eventually into the codebase."). Proposed content: a `triangles` layer, one PolygonZ per triangle in the run's CRS (z at each of the three corners, as the stopgap writes), with the attributes the run already knows: land-cover code and class name (the code system's names, as `rasputin palette` has them), mean z, plan area; a layer of the constraint polygons (the `--features` polygons clipped to the domain, with their code); optionally a QGIS style for land cover stored in the file (the `layer_styles` table), so the triangles open coloured. Land-cover colour: when the mesh carries CORINE classes, each triangle and each constraint polygon also gets its class's colour as a `#rrggbb` text column, so a reader can colour by the column without the stored style (in QGIS, a data-defined fill colour). The Graz file of 2026-10-05 carries `clc_color` on its `triangles` and `corine` layers; its stored styles categorise on `clc_code` and do not read the column. The colours are those of `rasputin palette` (`src_python/tin_engine/palettes.py`), the same 26 the Graz file carries: natural colours, as Ola chose over the official CLC legend in 16c's ruling 2 (that legend paints peat bogs blue and the sea almost white). **One attribute is not in the mesh yet**: the ID of the source polygon each triangle lies in (a sub-catchment cell, a parcel). Today only a class code labels triangles, one code system per mesh (16c, 16e's D1); the stopgap finds the cell by a centroid lookup outside rasputin. It needs a per-triangle array from a polygon source's ID property: its own small step, done just before this writer, because the `.vtk` and `.ply` outputs gain the attribute too (Ola, 2026-10-05, accepting the default: "defaults on the cell ID"). Written with the standard library's `sqlite3` beside the existing reader `src_python/tin_engine/io/geopackage.py`, which can read it back in the tests; no GDAL, OGR or Fiona (CLAUDE.md §2). Python only: the C++ core is unchanged and never sees the file (the I/O boundary). Prior art and working spec: Ola's stopgap `vtk_to_gpkg.py`, outside the repository (`rasputin_data/lands/scripts/`), which converts a mesh `.vtk` with `sqlite3`, NumPy, Shapely and PyProj, no GDAL, and its first use, the Mur above Graz (2 451 734 triangles, 10 m DEM, tolerance 10 m, 2026-10-05): **the GeoPackage was 677.9 MB against 110.6 MB for the binary `.vtk`**; the whole stopgap run took 39 s and 4.1 GB peak, and that includes the centroid cell lookup and the CORINE clipping, so it is not the cost of the GeoPackage encoding alone: the design measures the writer on its own. A GeoPackage row holds the triangle's whole ring (four corners, the first repeated) and a bounding box, and the file adds a spatial index, where `.vtk` stores each vertex once. Questions for the design: file size (keep the spatial index or not; at most an opt-in variant of 2D polygons with mean z as an attribute, proposed default: full PolygonZ; writing in chunks to bound memory); the CRS entry for a CRS with no EPSG code; where the run record (increment 25) goes; whether the constraint lines get a layer of their own | **merged (#231)** (branch `worktree-gpkg-out`: red `87f41c3d`, green `c59246e5`, fix-up `7085dddf`, 377 net production lines, mutation round `dddce8e8` ten of ten killed; `@perf`'s acceptance `57c25074` ACCEPTED: Numedalslagen 3.93 s / 203 MB, Romsdalen 19.4 s / 810 MB, every mesh byte-identical; code review round 1's one blocker, an empty land-cover layer crashing the write, fixed by `55cc3c3e` + `18442b27`, 378 net lines; code review round 2 APPROVED; merged 2026-10-12). Designed by `@architect`, 2026-10-11, on `81a551b3`; proposed by Ola 2026-10-05, "yes, roadmap row now, build after PR D"; PR D merged as #205, so the writer joins the encoders under `io/`). Measured on this design's probe: the quick check's Numedalslagen mesh (653,039 triangles) written in 4.0 s to 203 MB against 0.28 s and 29 MB for the binary `.vtk`, the stopgap 5.1 s; the Romsdalen slope mesh (3.1 million) in 21.5 s to 922 MB at 2.1 GB peak, the stopgap 28.7 s at 4.1 GB; the R-tree index is 60 % of the time; the same run writes the same bytes. Layers `triangles` (PolygonZ with code, name, colour, z_min, z_max, z_mean, area_m2, slope_deg), `lines`, `land_cover`, `land_cover_classes`, a `run` table with the run record, and the stored QGIS style; about 305 net lines, one PR; questions 1-5 answered with their defaults while Ola is away | `docs/increments/36-geopackage-output.md` | +| 37 | An MCP (Model Context Protocol) server over the rasputin commands, so an external agent (Codex, Claude Desktop, ChatGPT, Mistral) discovers and calls rasputin as tools instead of reading its help text: `rasputin mcp` serves over stdio, one tool per command with a JSON schema derived from the Typer command's own parameters (one source of truth, so schema and CLI cannot drift), each call run as a child `python -m tin_engine ` so a cancel kills it and a crash ends only the child, with the exit code, output paths, run record and `--stats` returned as data and a refusal returned as a structured error carrying the CLI's plain words and exit code (Ola, 2026-10-11: "my working name for what's we're doing is intent-driven triangulation. This means accessing the tools using agents and intent"; "Add an MCP server to the ideas file after GeoPackage"; 2026-10-12: "Rule the mcp SDK: yes, use it"). Built on the official `mcp` Python SDK 2.3 as an optional extra `rasputin[mcp]`; stdio first, Streamable HTTP later or never; no Tasks extension (the SDK has none server-side); a jobs cap, default 1; the GeoPackage output (36) is the mesh an agent reads | **built and review-approved** (branch `worktree-mcp-server`: design `b990e38c` approved in three review rounds, red `ee5b5d52` 62 tests, green `851592ba`, three code review rounds with a README section, an `error`-field fix and a help-text escape, master merged in as `030fd32b`, 328 net production lines against 260; `dev` pulls the `mcp` extra; next the push on Ola's yes). Designed (`@architect`, 2026-10-12, on `b4a81c0d`): about 250 counted lines, one PR; nine decisions with defaults for Ola (section 6); measured by the design's probe: a tool call costs 0.25 s over the command itself (the child's import of `tin_engine.cli`), the 1 m benchmark mesh 0.72 s through the tool, a cancelled child dead within 0.5 s over stdio and in process, unknown and mistyped arguments refused before any child runs; no refine or mesh code, so no `@perf` run | `docs/increments/37-mcp-server.md` | | — | A missing station number refused in plain words. In a reference-polygon file (`rasputin station-catchments --reference`), a feature whose `station` is null becomes the text "None" (`src_python/tin_engine/io/station_set.py@ed12512:89`). No station can match it, since a station number must look like `2.11.0` (`src_python/tin_engine/hydrography.py@ed12512:40`), so the polygon is dropped without a word and the station it belonged to is written with no comparison (`src_python/tin_engine/catchment_batch.py@ed12512:220`); two such polygons are refused as "station None appears twice". In a stations file a null number is refused, but in Pydantic's validation text rather than a sentence naming the file (`src_python/tin_engine/io/station_set.py@ed12512:79`). NVE's files have a number on every station, so only hand-made or third-party files are hit. The fix: both readers refuse a null `station`, naming the file; one or two lines. Plainer GeoJSON error messages in general are not planned until other people feed rasputin their own files | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | A GeoTIFF whose coordinate-system GeoKeys hold garbage refused in plain words. The reader turns each code key into a whole number with `int()`, and a key stored as a double or as text can hold NaN, infinity or letters. Probed 2026-10-06 on hand-made files: NaN or text in `ProjectedCSTypeGeoKey` (3072) stops `rasputin mesh --dem` with Python's conversion message (for NaN, "Invalid value for --dem: cannot convert float NaN to integer"), which names neither the key nor the file, and infinity ends in a Python traceback (`OverflowError`) (`src_python/tin_engine/io/geotiff.py@ed12512:171`). The same holds for every other code key the reader reads: 1025, 4099, 3076, 3074, 3075, 2048 and 2054 (`src_python/tin_engine/io/geotiff.py@ed12512:310`, `src_python/tin_engine/io/geotiff.py@ed12512:176`, `src_python/tin_engine/io/geotiff.py@ed12512:339`, `src_python/tin_engine/io/geotiff.py@ed12512:352`, `src_python/tin_engine/io/geotiff.py@ed12512:362`, `src_python/tin_engine/io/geotiff.py@ed12512:499`, `src_python/tin_engine/io/geotiff.py@ed12512:423`). Worse, a fractional value is cut to a whole number first: 2.5 in 4099 is refused as 2, and 32633.5 in 3072 is read as EPSG:32633 without a word (`src_python/tin_engine/io/geotiff.py@ed12512:321`); 1.5 in 1025, 9001.7 in 4099 and 9001.5 in 3076 are likewise accepted as 1 and 9001 (probed by code review round 1 on this row's branch, exit 0). The parameter keys are already checked (`_one_number`, `src_python/tin_engine/io/geotiff.py@ed12512:454`). The fix: one check for every code key, a whole number or a refusal naming the key and its value. No real file is known to do this | approved by Ola 2026-10-06; not designed (a small fix) | - | | — | tifffile's warning about the NoData tag silenced. On a float32 DEM whose `GDAL_NODATA` (42113) is the lowest float32 written in 17 digits (`-3.4028234663852886e+38`), tifffile prints "parsing GDAL_NODATA tag raised ValueError: ... is not castable to float32" to stderr when the file is opened (`src_python/tin_engine/io/geotiff.py@ed12512:146`). It is noise: rasputin reads the tag itself and accepts the value, since it is that float32 exactly (`src_python/tin_engine/io/geotiff.py@ed12512:535`). The fix: silence that one tifffile message while the reader opens the file, leaving every other tifffile warning visible | approved by Ola 2026-10-06; not designed (a small PR) | - | diff --git a/docs/increments/37-mcp-server.md b/docs/increments/37-mcp-server.md new file mode 100644 index 00000000..1516f842 --- /dev/null +++ b/docs/increments/37-mcp-server.md @@ -0,0 +1,804 @@ +# Increment 37: an MCP server over the rasputin commands + +**Status: built and review-approved** on branch `worktree-mcp-server` (red `ee5b5d52`, `@tester`, 62 tests; green `851592ba`, `@developer`, 322 net production lines by `python3 tools/count_loc.py b4a81c0d HEAD` against the estimate of 260 and the worst case of 360; no mutation round and no `@perf` run, section 10; section 12 is the as-built record; code review round 1 CHANGES REQUESTED, four blockers, all fixed: T5 case and prose `783c3368` (`@tester`), the `error` field `730162fe` (`@developer`, 323 net lines), the README section `19652b88` (`@architect`), two citations pinned `dbc77f3b`; round 2 CHANGES REQUESTED, two prose blockers, fixed: the README wording `2a871c50`, the help-text escape `01025786`; master `c317c7e4` merged in as `030fd32b` (328 net lines by `count_loc.py origin/master HEAD`), the one test it broke fixed `d8a1729d`; round 3 APPROVED; next the push on Ola's yes). Designed by +`@architect`, 2026-10-12, on `b4a81c0d` (branch `worktree-mcp-server`); the +nine defaults of section 6 were taken while Ola was away. Probe and its +output: `docs/increments/37-probes/`. Design review round 1 (Review section) +asked for three fixes, in sections 6.4, 6.5 and 7 and tests T4 to T6, T9 and +T11; after round 2, `@tester`'s red suite (section 9). + +## 1. What Ola asked + +An external agent (Codex, ChatGPT, Mistral, Claude Desktop) should discover +and call rasputin as tools instead of parsing its help text. Ola, 2026-10-11: +"my working name for what's we're doing is intent-driven triangulation. This +means accessing the tools using agents and intent. [...] How accessible is +the tooling to an agent like mistral or chatgpt?"; "Add an MCP server to the +ideas file after GeoPackage." The dependency is ruled: "Rule the mcp SDK: yes, +use it" (2026-10-12), so the server is built on the official `mcp` Python +SDK. + +The thing exposed is the CLI as it is today, one tool per command, with a +JSON schema per tool, structured errors (the plain-word refusal text plus the +exit code), and the run record, `--stats` and output paths returned as data. +The GeoPackage output (increment 36) is the mesh an agent can read. Nothing +in the server reaches the C++ core or widens the I/O boundary: the server +runs the CLI, and the CLI keeps every rule it has. + +The commands today (`src_python/tin_engine/cli.py@b4a81c0d:169`, listed by +`typer.main.get_command(app).commands` as `tests/python/cli_driver.py:42` +does): `version`, `draw`, `mesh`, `palette`, `fetch`, `fetch-stations`, +`catchment`, `station-catchments`. Their parameter counts, from the probe: +mesh 34 (36 after increment 26b merged, section 12), draw 10, fetch 10, catchment 11, station-catchments 11, +fetch-stations 3, palette 2, version 0. + +## 2. Prior art: legacy and literature + +*Literature.* The protocol is the Model Context Protocol, specification +revision **2026-07-28** (modelcontextprotocol.io/specification/2026-07-28; +schema `schema/2026-07-28/schema.ts`), JSON-RPC 2.0 messages over a +transport. What this design uses, by the specification's own sections: + +- *Server Features, Tools*: a tool is `name`, `description`, `inputSchema` + (JSON Schema, 2020-12 by default), optional `outputSchema` and + `annotations` (`readOnlyHint`, `destructiveHint`, `idempotentHint`, + `openWorldHint`). A result carries `content` blocks and optional + `structuredContent`, which must conform to `outputSchema` when one is + given. Two error channels: *protocol errors* (unknown tool, malformed + request; JSON-RPC error `-32602`) and *tool execution errors* ("input + validation errors", "business logic errors"; a result with + `isError: true`, which "clients SHOULD provide to language models to + enable self-correction"). Servers "MUST validate all tool inputs". Tool + names should use letters, digits, `_`, `-` and `.`. A tool "MAY return + links to Resources" as `resource_link` content. +- *Utilities, Progress*: a client that wants progress sends a + `progressToken` in `_meta`; the server MAY send `notifications/progress` + with `progress`, optional `total` and `message`; "the `progress` value + MUST increase with each notification, even if the total is unknown", and + it "MAY be floating point"; both sides "SHOULD implement rate limiting". +- *Utilities, Cancellation*: over stdio a client cancels with + `notifications/cancelled` naming the request id; the server SHOULD stop + processing, free resources and send no response; both sides handle the + race where the work finished first. Its *Timeouts* section: a sender + SHOULD time out a request and cancel it; "implementations MAY choose to + reset the timeout clock when receiving a progress notification + corresponding to the request", but "SHOULD always enforce a maximum + timeout, regardless of progress notifications". +- *Extensions, Tasks* (`io.modelcontextprotocol/tasks`): durable handles for + long operations, `tasks/get`, `tasks/cancel`, status `working`, + `completed`, `failed`, `cancelled`. An opt-in extension both sides must + negotiate; "host support varies by client". The installed SDK's client + dropped its experimental task support in v2 (migration guide, "Experimental + Tasks removed") and the low-level server has no task handler, so Tasks are + out of this increment (section 6.5). + +The SDK is `mcp` **2.3.0** (PyPI; MIT; `Requires-Python >=3.10`; +py.typed), measured in the probe venv. Its v2 facts this design rests on +(py.sdk.modelcontextprotocol.io/v2, confirmed by introspection in the probe): +`FastMCP` is renamed `MCPServer`; the low-level `Server` takes handlers as +constructor arguments (`on_list_tools`, `on_call_tool`, each +`async (ctx, params) -> result`) and runs over `mcp.server.stdio.stdio_server()`; +`Client(server)` connects in process for tests, `Client(StdioServerParameters( +command, args, env, cwd))` spawns a stdio server, and `Client("http://...")` +speaks Streamable HTTP; `Client.call_tool(name, arguments, +read_timeout_seconds, progress_callback)`; `ctx.session.send_progress_notification( +progress_token, progress, total, message)`; `ctx.meta["progress_token"]` is +the token (the wire key is `progressToken`); an `MCPError` raised in a +handler becomes a JSON-RPC error, any other exception "Internal server +error"; an async handler's `await` is cancelled when the client cancels +(the SDK's cancellation page), a sync handler is not. Wire fields are +snake_case in Python (`input_schema`, `is_error`, `structured_content`). +Two facts the probe found that the documentation does not say: **the SDK +validates no tool argument on the server side** (its `jsonschema` use is in +the client; `MCPServer` accepted an unknown argument and ran the tool, +probe run 1), and **the in-process transport carries no progress token** +(`mcp/shared/jsonrpc_dispatcher.py` sets it; the in-process +`direct_dispatcher` does not; probe part E, "progress ticks 0" in process, +1 over stdio). The SDK's dependency set is 13 packages (`anyio`, `httpx2`, +`jsonschema`, `mcp-types`, `opentelemetry-api`, `pydantic`, `pyjwt[crypto]`, +`python-multipart`, `sse-starlette`, `starlette`, `typing-extensions`, +`typing-inspection`, `uvicorn`), none on CLAUDE.md §2's list; importing +`mcp.server.lowlevel` and `mcp.client` costs 0.40 s (probe E). + +Other geospatial tools over MCP, as found (sparkgeo's tracked list, +`github.com/willcadell/geo-mcp-servers`, "100 servers tracked" on +2026-10-12): `gdal-mcp` +(`github.com/JordanGunn/gdal-mcp`): one tool per operation (`raster_info`, +`raster_reproject`, `vector_clip`, ...), FastMCP 2.0 over stdio, built on +Rasterio, GeoPandas and pyogrio, which this project may not use; the QGIS +plugins (`qgis_mcp_plugin`, 118 tools; `qmcp`) run a socket server inside +QGIS Desktop. None wraps an existing CLI with schemas derived from it; all +hand-write their tool signatures. This design differs in that one respect +only, and claims no novelty: it is an adapter to a published protocol. + +Typer's own switch for plain errors, used by the server for the child +process: `TYPER_USE_RICH` (Typer 0.27.3, `core.py` line 27 of the package, +`HAS_RICH = parse_boolean_env_var(os.getenv("TYPER_USE_RICH"), default=True)`). +With `TYPER_USE_RICH=0` a refusal is `Usage: ...`, `Try ... --help`, a blank +line and one `Error: ` line (probe; `_TYPER_STANDARD_TRADITIONAL` has +no effect in this version). + +*Legacy.* Nothing. The archived tree has no MCP, JSON-RPC or JSON Schema +code: + + $ git grep -n -i -E "mcp|model context protocol|json-?rpc|json schema|jsonschema" legacy-archive -- legacy + (no files; exit 1) + +Its only servers are a three.js web viewer (`legacy/rasputin/web_visualize.py`, +`git grep -l -i -E "server|rpc" legacy-archive -- legacy` also lists +`solar_position.h`, a false hit on "observer"); neither carries across. + +*In this tree.* `tests/python/cli_driver.py:42-56` already derives the +command set from the Typer app and strips Rich's boxes from refusals; the +server does the first the same way and avoids the second with +`TYPER_USE_RICH=0`. `tools/bench.py:45-90` runs the CLI as a child through a +`Runner` protocol with an injected `SubprocessRunner`; the server's child +runner follows that shape. `src_python/tin_engine/run_record.py@b4a81c0d:288` +(`as_json`) is the record the server returns. + +## 3. Measured on this worktree (`docs/increments/37-probes/mcp_probe.py`) + +AC power, Python 3.14.7, mcp 2.3.0, typer 0.27.3, pydantic 2.14.1; the +probe's own venv, `tin_engine` from this worktree with the `_core` built at +`46b5b1cb` (no C++ changed since: `git diff --stat 46b5b1cb HEAD -- include +src bindings CMakeLists.txt` is empty); `@developer` was building in another +worktree, so timings are medians of 3 to 5 on a shared machine. The full +output is `docs/increments/37-probes/mcp_probe_output.txt`. + +The probe is the design in miniature: a low-level `Server` with one tool per +Typer command, each schema derived from the command's click parameters, +arguments validated by the derived Pydantic model, the command run as a child +process. + +| what | measured | +|---|---| +| schemas derived from all 8 commands | 16,575 bytes of JSON; `mesh` 34 properties, `required: ["out"]`, 8,269 bytes | +| parameters needing a note | 2 of 81 (at `b4a81c0d`; 83 after increment 26b, section 12): `snap_spacing` on `draw` and on `mesh` has a validating callback (`_snap_spacing`, `src_python/tin_engine/cli.py@b4a81c0d:478`, `:676`), which stays in the child and still refuses there | +| tuple options | `bbox` -> `prefixItems` of four numbers, `minItems`/`maxItems` 4; `tolerance_near` -> `[string, number]` | +| flag pairs (`--binary/--ascii`, `--delaunay/--no-delaunay`, `--[no-]features-merge-same-class`) | one boolean each, default from the CLI (`binary: true`) | +| `fetch --connections` (an integer range) | `{"type": "integer", "minimum": 1, "default": 8}` | +| refusal, missing `--out` | caught by the derived model before any child runs: `is_error: true`, `"out: Field required"` | +| refusal from the child (bad `--dem`) | `is_error: true`, `{"exit_code": 2, "error": "exit 2: Invalid value for --dem: cannot read /nonexistent.tif: No such file or directory"}` | +| wrong type, unknown argument | `"name: Input should be a valid string"`; `"bogus: Extra inputs are not permitted"` (`additionalProperties: false`) | +| unknown tool | JSON-RPC error `-32602`, "Unknown tool: nosuch" | +| client timeout 1.0 s on a tool awaiting a child `sleep 30` | client raises after 1.00 s; the child is dead 0.5 s later, in process and over stdio | +| two concurrent `version` calls | 0.29 s together (one alone 0.26 s): they ran side by side | +| tool round trip, `version` (child inside) | in process 0.261 s; over stdio 0.264 s; stdio connect and initialize 0.465 s | +| `rasputin mesh` on the 1 m benchmark (`tests/fixtures/dem_archive/7908_3_10m_z33.tif`, 5051 x 5051 nodes at 10 m, `--tolerance 1`) | child 0.72 s (0.72, 0.72, 0.72); 464,290 triangles; binary `.vtk` 16.9 MB; record 1,583 bytes, 38 keys | +| the same through the tool | 0.715 s in process, 0.719 s over stdio; result carries the record (38 keys), `outputs` with the mesh path, one stderr line; the text block 1,958 characters | +| progress ticks for that run | 0 in process (no token), 1 over stdio (the one stderr line, the summary at the end) | +| where the child's time goes | `python -c pass` 0.019 s; `import tin_engine.cli` 0.247 s; `rasputin version` 0.262 s as a child, 0.0107 s in process (`CliRunner`) | + +So a tool call costs about 0.25 s over the command itself, almost all of it +importing `tin_engine.cli` (which imports `_core`) in the child; against a +mesh of seconds to minutes this is noise, against `version` it is the whole +call. Section 6.4 takes the child route anyway and says why. + +## 4. Blueprint: components and data flow + +``` +agent (Codex, Claude Desktop, ...) stdio: JSON-RPC 2.0 lines + | tools/list, tools/call ----------------------------------> rasputin mcp + | | + | tin_engine.mcp.server (L5) + | - one Tool per Typer command, from + | tin_engine.mcp.schema (click params -> + | Pydantic model, extra forbidden, and back to argv) + | - validates arguments with that model + | - runs the child, kills it on cancel + | - shapes the result; maps exit 2 to is_error + | | + | | argv, env TYPER_USE_RICH=0 + | v + | python -m tin_engine ... (a child process: + | cli.py as today; --record to a sidecar the only + | file beside --out, read back and removed) + | | place _core + | <--- result: exit_code, outputs, record, ------+ stdout lines, stderr lines, runs + stats, stderr; or is_error + words exit code, the record JSON +``` + +Boundaries: + +- **The server never imports `_core`, `raster`, `io/` or any pipeline.** + Its first-party imports are `tin_engine` (for `installed_version`) and + nothing else; it takes the Typer app as an argument, so the schema layer is + tested on a toy app without the kernel (section 9, T1 to T4). This is the + Pybind11 firewall unchanged: `_core` runs in the child, behind the CLI. +- **The CLI is the contract.** The server adds no option, no default, no + validation of its own beyond the schema the CLI implies; a rule the CLI + enforces (paths under `--out-parent`, `--domain` needs `--tolerance`, the + prohibited-dependency and I/O boundaries) holds because the CLI runs. A + new command or option appears as a tool or a property with no server + change; the schema and the CLI cannot drift because the schema is read + from the CLI at start. +- **Data, not execution.** `tin_engine.mcp.schema` is pure: Typer command in, + Pydantic model and `Tool` out, and arguments in, argv out. No I/O, no + async. `tin_engine.mcp.server` holds the only I/O (the child, the sidecar + record) and the only async code. +- **The server's stdout is the wire.** Over stdio the server process writes + nothing but JSON-RPC to its own stdout: the child's stdout and stderr are + pipes the server reads, and anything the server has to say for itself + (an unexpected exception's traceback) goes to its stderr. A stray + `print` in the server would corrupt the protocol stream, so `server.py` + has none, and T7 (a real stdio round trip) is where one would show. +- **Async-ready.** The server is `anyio`/`asyncio` throughout: the child is + `asyncio.create_subprocess_exec`, its streams are read as tasks, a second + call does not wait for the first unless the jobs cap says so, and the same + `Server` object could be mounted in Streamable HTTP later without change. + +## 5. The tool surface + +One tool per command, named as the command with `-` as `_` (`fetch_stations`, +`station_catchments`): the spec allows `-`, but most clients and models +write tool names as identifiers. `rasputin mcp` itself (section 6.8) is +excluded from the list. Tool order is the CLI's (deterministic, as the spec +asks). + +Per tool: + +- `description`: the first paragraph of the command's docstring, as Typer + shows it in `rasputin --help`. +- `input_schema`: the derived model's JSON Schema with `title` removed and + `additionalProperties: false`. Each property carries the option's help + text as `description`, its default, and its type by this table, which the + probe exercised on all 81 parameters at `b4a81c0d`: click `str`/`path` -> string; + `int` -> integer (`IntRange` adds `minimum`/`maximum`); `float` -> number; + `bool` flags -> boolean (a `--x/--no-x` pair is one boolean; `true` emits + the first name, `false` the second); a tuple type -> fixed-length array + (`prefixItems`); `multiple=True` -> array of the item type; a `Choice` + -> `enum`; an option with default `None` -> nullable, not required; + required options and arguments -> `required`. Paths are strings: the + agent gives paths on the machine the server runs on, as the CLI does. +- `output_schema`: one `RunResult` model for every tool (below), so clients + can validate. +- `annotations`: `version` `read_only_hint=True, idempotent_hint=True`; + `fetch` and `fetch_stations` `open_world_hint=True` (network); every tool + `destructive_hint=False` (the CLI never deletes; it refuses to overwrite a + mesh with a report, `src_python/tin_engine/cli.py@b4a81c0d:1335-1364`). + +`RunResult` (Pydantic, `tin_engine.mcp.server`): + +| field | meaning | +|---|---| +| `command` | the argv the server ran, as `shlex.join` prints it (what `--record`'s `command` field says, `src_python/tin_engine/cli.py@b4a81c0d:1367`) | +| `exit_code` | the child's exit code (0 here; a refusal is an error result, below) | +| `seconds` | wall time of the child | +| `outputs` | the stdout lines that are paths of files that exist after the run (mesh: the mesh, `--out-edges`, `--stats` file, `--record` file; catchment: the polygon; fetch_stations, station_catchments: directories) | +| `stdout` | every stdout line (fetch's report, palette's preset text, version's two lines) | +| `stderr` | every stderr line (the mesh summary, feature counts, warnings) | +| `record` | for `mesh`: the `--record` JSON as an object (38 keys on the fixture), whether or not the caller asked for a record file; when the caller did not, it is read from a sidecar file beside `out` (section 6.4) and the sidecar is removed | +| `stats` | for `mesh` with `stats` given: the Markdown report text; `stats: "-"` is redirected to a sidecar file beside `out` and returned here rather than mixed into stdout | + +The `content` of a successful result is one text block with the same JSON +(the spec's backward-compatible form) plus one `resource_link` per output +(`file://` URI, the file's suffix as `mimeType` where known: `.gpkg` +`application/geopackage+sqlite3`, `.vtk` and `.ply` `application/octet-stream`, +`.geojson` `application/geo+json`, `.json` `application/json`, `.md` +`text/markdown`). A large mesh never travels through the protocol: the +agent reads the GeoPackage with its own tools. + +Errors, following the spec's two channels: + +| case | result | +|---|---| +| arguments fail the derived model (missing required, wrong type, unknown key) | `is_error: true`; text and `structured_content` `{"exit_code": null, "error": ": ; ..."}`; no child runs | +| the child exits 2 (a `BadParameter`, `src_python/tin_engine/cli.py@b4a81c0d:416-1177`) or 1 (`typer.Exit(1)` after a fetch or service error, `src_python/tin_engine/cli.py@b4a81c0d:1527`, `:1566`, `:1569`, `:2560`) | `is_error: true`; `{"exit_code": 2, "error": "exit 2: "}`; `stderr` kept in full | +| the child dies of a signal | `is_error: true`; `{"exit_code": -N, "error": "killed by signal N ()"}` | +| unknown tool | `MCPError(-32602, "Unknown tool: ")`, a protocol error, as the spec files it | +| the server's own fault (an exception in the handler) | caught and returned as `is_error: true` with `exit_code: null` and the exception's words; the SDK's "Internal server error" is never the answer | + +The refusal words are the child's `Error:` line(s) with `Usage:` and +`Try ... --help` dropped; measured: "Invalid value for --dem: cannot read +/nonexistent.tif: No such file or directory". They are the CLI's words, so +increment 25's plain-output rules carry over for free. + +## 6. Decisions, each with its default for Ola + +### 6.1 Dependency: an optional extra (default), not required + +`pyproject.toml` gains `mcp = ["mcp>=2.3"]` under +`[project.optional-dependencies]` beside `codecs`, `viewer` and `dev` +(`pyproject.toml@b4a81c0d:48-70`). A mesh-only install is unchanged; `pip +install "rasputin[mcp]"` adds the SDK's 13 packages (section 2), two of them +HTTP servers this increment never starts. `tools/check_prohibited_deps.py` +scans every extra's entries (`tools/check_prohibited_deps.py@b4a81c0d:204-216`, +`for extra in project.get("optional-dependencies", {}).values()`), so the +new extra is checked and passes: none of the 14 names is on the list. +`@developer` runs the checker after the edit, and plants `rasterio` in the +new extra once to see it fail (the required-reading rule on verifying a +gate). CI's Python jobs install `.[dev]` today, the `python` matrix job at +`.github/workflows/main.yaml:261` and the extras job at `:316`, `:337`, +`:351`; all four lines add `mcp` to the extras so the suite runs there; +locally the suite `pytest.importorskip("mcp")`s. +mypy: both `mcp` and `mcp_types` ship `py.typed`, so no override. + +### 6.2 Transport: stdio now; HTTP is one later flag, not designed here + +Stdio is what Claude Desktop, Codex and Claude Code launch (a command and +arguments in a JSON config, section 6.8). Streamable HTTP is what a hosted +ChatGPT connector needs; it is the same `Server` object behind +`mcp.server.streamable_http_manager`, with authentication and a listening +port to think about, so it is its own small increment if Ola wants ChatGPT +in particular. Nothing here forecloses it. + +### 6.3 Schema source: the Typer command's click parameters + +Measured against the alternative. The click parameters of all eight commands +give name, type, default, required, help, repetition and tuple arity for 81 +of 81 parameters (at `b4a81c0d`; the count moves with the CLI, section 12); the one note is a validating callback, whose refusal still +comes back from the child in words. A Pydantic model per command would be +a second statement of the same 81 parameters (about 34 fields x 3 lines for +`mesh` alone) that drifts the first time someone adds an option to one and +not the other; the drift test it would need is the derivation itself. So: +`tin_engine.mcp.schema.derive(app) -> list[DerivedCommand]`, where a +`DerivedCommand` is the click command, its tool name, its Pydantic model +(`create_model`, `ConfigDict(extra="forbid")`) and `argv(args) -> list[str]`, +which emits only the keys the caller set (`model_fields_set`), so every CLI +default stays the CLI's. + +What is lost, by design: option grouping (`rich_help_panel`), `metavar` +(`BOX`, `FILE N`) and the validator callback's rule are not in the schema; +the help text is the whole documentation of a property. The remedy for weak +help text is better help text in `cli.py`, which then serves both readers. + +### 6.4 Execution: a child process per call + +In-process invocation is 25 times cheaper (0.0107 s against 0.262 s for +`version`) and was rejected for five reasons, each measured or inherent: +(1) a mesh runs C++ for seconds to minutes with the GIL held in stretches; +a sync handler cannot be cancelled (SDK cancellation page), an async one +cannot interrupt a C++ call; the child is killed within 0.5 s of the cancel +(probe C). (2) Two agents' calls, or one agent's two calls, run as two +processes with their own memory and no shared `sys.stdout`; in process the +CLI's `typer.echo` to the process's stdout would have to be redirected per +call, which is process-global. (3) A crash in `_core` (a bounds check, an +allocation failure at basin scale) ends the child and returns an error; in +process it ends the server. (4) Memory from a basin run is returned to the +system when the child exits; the server stays small. (5) The result is the +exact CLI behaviour, including `sys.argv`-derived fields in the record. +The cost, 0.25 s a call, is importing `tin_engine.cli` in the child; it is +paid once per tool call and is below the noise of any mesh. + +The child is `[sys.executable, "-m", "tin_engine", *argv]`: the same +interpreter and the same installed `tin_engine` as the server, found without +`PATH`. That needs `src_python/tin_engine/__main__.py` (new, 3 lines: +`sys.argv[0] = "rasputin"` so the record's `command` field reads +`rasputin mesh ...`, then `app()`); `python -m tin_engine version` becomes a +second way to run the CLI. The child's environment is the server's plus +`TYPER_USE_RICH=0`, `NO_COLOR=1`, `TERM=dumb`; `RASPUTIN_DATA` and the cache +default pass through unchanged. +The child is started without `cwd=`, so it inherits the server's working +directory: a relative `out` and its sidecar beside it resolve to the same +place in the child and in the server's read-back and `unlink`. + +*The sidecar files.* For `mesh` without `record_path`, the server appends +`--record ` and reads it back; with `stats: "-"` it appends +`--stats ` instead and returns the text; both sidecars are removed +after the read (`unlink(missing_ok=True)`, in the handler's `finally`, so a +refused or killed run leaves none). The sidecar is **beside `out`**, not in +the system temp directory: `Path(out).parent / f".{Path(out).name}.record- +{token}.json"` and `...stats-{token}.md`, `token` eight hex digits from +`secrets.token_hex(4)`, the name chosen by the server and the file created +by the child. Checked against `_destination` +(`src_python/tin_engine/cli.py@b4a81c0d:420-447`), which every `--record` +and `--stats` path passes through (`_report_target` at `:1335-1347`, +`_record_target` at `:1350-1364`, both called at `:1195-1196` after `--out` +at `:1180`): + +- `out.is_symlink()` (`:438`): a fresh random name is not a symlink. +- containment in `out_parent` (`:441-444`): `Path(record).resolve()` is + inside `out_parent.resolve()` exactly when `Path(out).resolve()` is, since + the two share a directory and `out` is not a symlink (refused at `:438` + before this); so a system temp path (`/var/folders/...`) would be refused + whenever `out_parent` is set, and the sidecar beside `out` never is. +- `target.parent.is_dir()` (`:445`): the sidecar's directory is `out`'s, + which `_destination(out, ...)` at `:1180` has already accepted, or refused + first naming `--out`; the server never creates the directory or the file. +- `target in taken` (`:1343`, `:1360`): the random name collides with no + mesh, edges or report path. + +The sidecar is created by the child with the child's permissions in a +directory the caller already chose to write a mesh into, so it widens no +boundary. A caller who wants the record kept passes `record_path`, which +is then the caller's path and not removed. The runner is injected (`Child` +protocol: argv prefix in, a `(returncode, stdout lines, stderr lines)` +out), so tests substitute a fake child (section 9). + +### 6.5 Long runs: one blocking call with progress; cancellation kills the child; no Tasks + +A basin mesh takes minutes and the call stays open for them. Over stdio the +server sends `notifications/progress` when the client gave a token: one per +stderr line the child prints (message: the line), and a heartbeat every 5 s +with message `"running, N s"` when no line came, so a client that resets +its timeout on progress (the spec's Cancellation page allows it) does not +give up on a silent run. Measured: `mesh` prints nothing until its summary +line (probe E: one stderr line in 0.72 s), so on today's CLI the heartbeat +is the only progress a long mesh shows; feature and tolerance runs print a +few lines as phases pass (`src_python/tin_engine/cli.py@b4a81c0d:1084`, +`:1708`, `:1783`, `:1810`, `:1916`). + +*The `progress` value.* The spec says it MUST increase with each +notification; a line count would not, since a heartbeat has no line. So +every notification of a call, line or heartbeat or waiting (section 6.6), +carries `progress` = seconds elapsed since the handler began, from +`time.monotonic()`, rounded to milliseconds, and `total` omitted (the run's +length is unknown). One sender per call holds the last value sent and sends +`max(elapsed, last + 0.001)`, so two lines read in the same millisecond +still increase; the 0.001 is the rounding step, and the sender is one +`asyncio` task per call (the stderr reader and the heartbeat timer feed the +same sender), so the values are never sent out of order. The probe's +version (`docs/increments/37-probes/mcp_probe.py:186-188`) sent the line +count; its measured output, the count and text of the ticks, is unchanged +by this choice, so the probe was not rerun. + +The constant 5 s assumes runs from seconds to an hour (12 to 720 +notifications) and was checked at the fixture's 0.72 s run (zero +heartbeats) and the probe's 1 s cancellation; it is a module constant, not an +option. A client's own timeout is the client's; the SDK's client default is +none. + +Cancellation: the SDK cancels the handler's `await` on `notifications/cancelled` +(stdio) or on a dropped stream; the handler's `finally` kills the child and +waits for it, then returns nothing (the spec says no response). + +*What a killed run leaves on disk.* The CLI writes a GeoPackage as +`.part` and renames it at the end +(`_write_geopackage`, `src_python/tin_engine/cli.py@b4a81c0d:1250-1289`, +the `os.replace` at `:1289`), +so a killed `.gpkg` run leaves at most a `.part` file, which the next run +replaces. `.vtk` and `.ply` are not written that way: the file is encoded in +memory and written with one `target.write_bytes(data)` +(`src_python/tin_engine/cli.py@b4a81c0d:1241`), and `_writing` (`:2121`) +only turns an `OSError` into a refusal, it does not rename. So **a `mesh` +call cancelled during that write can leave a truncated `.vtk` or `.ply` +under the asked name**; cancelled before it, the old file (if any) is +untouched, and cancelled after it, the mesh is complete and only the report +or record is missing. The server does not guard against this: it cannot +tell a truncated file from a complete one or from the caller's older file +under that name, and deleting on cancel would destroy the last two. It +accepts and documents it instead: a cancelled call returns no result, so +the agent holds no path it was told to trust, and the next run overwrites +the file. The remedy is in `cli.py`, every output through `.part` and a +rename; it is in Ola's ideas file and is not this increment's. The server's +`instructions` (section 6.8) gain one line: "a cancelled `mesh` may leave an +unreadable `.vtk`/`.ply` at `out`; rerun before reading it". + +The Tasks extension is the right shape for hour-long runs with +reconnects, and is out: the SDK's v2 client removed its experimental task +API, the low-level server has no task handler, and client support "varies". +When the SDK ships it, a task is a wrapper round the same child runner. + +### 6.6 Concurrency: a jobs cap, default 1 + +Two calls run as two children (probe D), and two basin runs are two to six +GB of memory (`docs/increments/23-basin-scale.md`'s limits). `rasputin mcp +--jobs N` (default 1) is an `anyio.Semaphore`; a call beyond the cap waits, +with heartbeats every 5 s saying `"waiting for a running job, N s"` and the +same elapsed-seconds `progress` as section 6.5 (the clock starts when the +handler begins, before the wait, so the first running notification is above +the last waiting one), and is still cancellable while waiting. `version` and `palette` also wait: a special +case for cheap commands is a second rule to explain for a 0.3 s gain. The +default was checked at two `version` calls; an agent wanting parallel +fetches raises it. + +### 6.7 Resources: none listed; outputs are resource links and paths + +No `resources/list`: a server that lists every mesh it ever wrote needs a +registry and a retention rule, and a 17 MB to 1 GB mesh does not belong in a +context window. The run record is in the result; each output file is a +`resource_link` with a `file://` URI and a path in `outputs`, which the agent +reads with its own tools (sqlite3 on the GeoPackage, the Gemini note's +`rasputin://meshes/{id}` is the idea this does not take up). + +### 6.8 Where it lives and how it starts + +`src_python/tin_engine/mcp/` with `__init__.py` (empty), `schema.py` and +`server.py`; `tests/python/test_layering.py` gains rows in L5 (`cli` row adds +`mcp.server`; `mcp` ``; `mcp.schema` ``; `mcp.server` `mcp.schema tin_engine`; +`__main__` `cli`), all sideways or down; `test_the_table_names_every_module` +(`tests/python/test_layering.py@b4a81c0d:143-149`) demands them. + +Start: `rasputin mcp [--jobs N]`, a Typer command in `cli.py` that imports +`tin_engine.mcp.server` inside the function and, when the import fails, +refuses with exit 2 and `Error: the mcp extra is not installed; run +pip install "rasputin[mcp]"`. A subcommand is chosen over a console script +because it is where an agent reading `rasputin --help` finds it, and the +install story stays one package. The server excludes its own command from +the tool list. A client configuration, which README gains in a short +"Connect an agent" section with the Claude Desktop and Codex forms: + +```json +{"mcpServers": {"rasputin": {"command": "/path/to/venv/bin/rasputin", "args": ["mcp"]}}} +``` + +The server's `instructions` (the initialize result's field, shown to the +model) are six lines: paths are on this machine; `mesh` writes a file and +returns its record; write `.gpkg` to inspect a mesh; `fetch` first for a +catalogue source; a refusal's words say what to change; a cancelled `mesh` +may leave an unreadable `.vtk`/`.ply` at `out` (section 6.5). + +### 6.9 Tool names: underscores + +`fetch-stations` is tool `fetch_stations`. Default yes; the alternative, +the hyphenated name, is spec-legal but some clients reject it. + +## 7. Degeneracy policy + +| input | behaviour | +|---|---| +| malformed arguments (missing required, wrong type, unknown key, wrong arity for `bbox`) | refused by the derived model before any child runs; `is_error` with Pydantic's words per field (measured, section 3) | +| a missing data folder (`--cache` default with `RASPUTIN_DATA` unset, a `--dem` directory that is not there) | the child refuses with its words and exit 2; the server passes them through; it never guesses a path | +| a refused command (any `BadParameter`, any `typer.Exit(1)`) | `is_error: true`, exit code and words; stderr in full | +| a killed run (client cancel, client gone, server stopped) | the child is killed and waited for within the cancel; no response; the sidecar is removed; a `.gpkg` run leaves at most a `.part` file, a `.vtk`/`.ply` run killed during its one `write_bytes` leaves a truncated file under the asked name (section 6.5) | +| `mesh` with `out_parent` set | the sidecar record or report is beside `out`, so `_destination` accepts it exactly when it accepts `out`; the call succeeds, or is refused naming `--out`, never naming `--record` or `--stats` | +| a child that prints a non-UTF-8 byte | decoded with `errors="replace"`; never a server fault | +| a child whose stdout path line is not a file (fetch's report lines) | kept in `stdout`, not in `outputs` | +| two concurrent calls | both accepted; the second waits for the semaphore (jobs 1) with heartbeats, or runs beside the first (jobs 2+) | +| the client never sends a progress token | no notifications; the heartbeat task is not started | +| a tool call for `mcp` itself | not a tool: unknown tool, protocol error | +| the `mcp` extra absent | `rasputin mcp` exits 2 with the install line; every other command unchanged | +| an exception inside the server's own code | `is_error: true` with the words, `exit_code: null`; the server keeps serving | + +## 8. Left out, with reasons + +- **An intent parser (`parse_mesh_intent`) and a `MeshIntent` schema.** The + agent is the intent parser; rasputin gives it the typed tools. The Gemini + note's version (unverified, in Ola's private ideas folder) is a second + schema beside the CLI's; this design has one. +- **An OpenAPI export.** Cheap once `schema.py` exists (the same models' + JSON Schema under an OpenAPI 3.1 envelope), and useful for ChatGPT Actions + and function calling; its own small increment, after this one proves the + derivation. Handed back as an idea. +- **Streamable HTTP, authentication.** Section 6.2. +- **Tasks.** Section 6.5. +- **Listed resources, a mesh registry.** Section 6.7. +- **Phase progress from `mesh`.** The CLI is silent until its summary; a + `--progress` or verbose phase line per `PhaseClock` phase would make the + heartbeat informative. Handed back as an idea; a `cli.py` change for + humans and agents alike, not an MCP change. +- **Rewriting tool descriptions.** They are the commands' docstrings, which + reference increments ("increment 16c"); the product review in Ola's ideas + file covers that rewrite, and it will reach the tools through the + derivation. + +## 9. Tests (`@tester`), red first + +All in `tests/python/test_mcp_*.py`, `pytest.importorskip("mcp")` at module +top; the SDK's `Client` runs the server in process except where stdio is +the point. Fixtures and their figures as measured in section 3. + +- **T1 schema from a toy app** (no kernel): a Typer app with two commands + covering every row of section 5's type table (str, path, int, IntRange, + float, flag pair, 4-tuple, 2-tuple of path and float, `multiple`, Choice, + nullable, required argument) derives the expected properties, `required`, + `additionalProperties: false`, defaults and descriptions; the tool name + maps `-` to `_`; the server's own command is excluded. +- **T2 argv round trip on the toy app**: for each case, `argv(model(args))` + is the expected command line: only set keys appear; a `false` flag emits + the second name; a tuple expands; a list repeats the option; an argument + is positional. +- **T3 the real app**: `derive(app)` yields the eight commands with their + parameter counts (34, 10, 10, 11, 11, 3, 2, 0; as built the suite pins the + tool names and the equality with the click parameters instead, section 12) + and `mesh`'s `bbox`, + `tolerance_near`, `binary`, `features`, `snap_spacing` schemas as section + 3 shows; the whole set under 20,000 bytes (16,575 measured) so an agent's + tool list stays small. +- **T4 validation before the child**: missing `out`, `tolerance: "ten"`, an + unknown key, `bbox` of three numbers: `is_error`, the field named, and the + fake child never called. And the argv the fake child receives for `mesh` + with `out: /a.vtk` and `out_parent: `: `--out-parent ` is + passed through, `--record` names a file in `` whose name starts with + `.a.vtk.record-` and ends `.json`; with `stats: "-"` the `--stats` value + is `/.a.vtk.stats-<8 hex>.md` and no `--record` is added when + `record_path` is given. +- **T5 refusal mapping**: a fake child returning exit 2 with + `Usage:`/`Try`/`Error: Invalid value for --dem: ...` lines gives + `{"exit_code": 2, "error": "exit 2: Invalid value for --dem: ..."}`; exit + 1 likewise; a signal exit gives `killed by signal`. After a fake child that + exits 2 without creating the sidecar, and after one that creates it and + exits 2, no `.a.vtk.record-*` file remains in ``. +- **T6 a real `mesh`** through the tool on the micro tile + (`tests/python/geotiff_fixtures.py:97-105`, 3 x 4 cells at 10 m x 5 m, + no codec needed): exit 0, `outputs` is the `.vtk`, `record` is a dict with + the record's keys (`run_record.WORDING`'s order), the sidecar record file + is gone (the directory holds the `.vtk` and nothing else), the `.vtk` + parses with `vtkread`; the same call with `out_parent` set to the `.vtk`'s + directory succeeds the same way (the case review round 1 found: a sidecar + in the system temp directory would exit 2 here with "is outside the + permitted parent"), and with `out_parent` set to a sibling directory it is + `is_error` naming `--out`, not `--record`; with a relative `out` under + `monkeypatch.chdir` the mesh and the sidecar's removal land in that + directory (the child inherits the server's cwd); `outputs` is exactly the + `.vtk`, so the echoed sidecar path (`src_python/tin_engine/cli.py@b4a81c0d:1247`) is excluded; + with `stats: "-"` the result's + `stats` starts with the report's heading and stdout carries no Markdown; + and once on the Kartverket fixture under `needs_codecs` with + `--tolerance 1`: 464,290 triangles in the record, 0.7 s on the probe's + machine. +- **T7 stdio round trip**: `Client(StdioServerParameters(sys.executable, + ["-m", "tin_engine", "mcp"]))` lists the eight tools and calls `version`; + the result's `stdout` has two lines and `exit_code` 0. Connect and + initialize measured at 0.465 s, so the test's budget is generous. +- **T8 cancellation**: the fake child is `sys.executable -c "import time; + time.sleep(30)"` with its pid written to a file; `read_timeout_seconds=0.5` + raises at the client and the pid is gone within 1 s (measured: dead 0.5 s + after a 1.0 s timeout, both transports); over stdio as well, since that is + the transport the cancel notification travels on. +- **T9 progress over stdio**: a fake child printing three stderr lines 0.2 s + apart gives three progress notifications with the lines as messages; the + heartbeat constant monkeypatched to 0.1 s on a child that prints one line + at 0.25 s and exits at 0.6 s gives at least three `"running, N s"` + heartbeats around that line; over the whole sequence, lines and heartbeats + together in the order received, every `progress` is strictly greater than + the one before, each is within 0.05 s of the wall time since the call + began (so the value is elapsed seconds, not a count), and `total` is + absent. With jobs 1 and a first call holding the semaphore, the second + call's `"waiting for a running job, N s"` heartbeats and its later running + notifications form one strictly increasing sequence. In process no token + arrives (the SDK's gap, measured) so the in-process case asserts only that + the run completes. +- **T10 jobs cap**: with jobs 1, two calls on a 0.5 s fake child complete + in about 1 s with non-overlapping child intervals; with jobs 2 in about + 0.5 s. +- **T11 `rasputin mcp` without the extra**: the import made to fail + (`monkeypatch.setitem(sys.modules, "mcp", None)`, and + `monkeypatch.delitem(sys.modules, "tin_engine.mcp.server", raising=False)` + so an earlier test's import of the server does not satisfy the lazy + import), exit 2 and the install line; `rasputin --help` still lists + `mcp`. +- **T12 layering**: the new rows (section 6.8) pass `test_layering.py` + unchanged in rule; `test_the_table_names_every_module` sees `__main__`, + `mcp`, `mcp.schema`, `mcp.server`. + +Mutation testing is not required: no suite here is invariant-critical in the +README's sense (no topology); the one place a typo hides, the type table of +6.3, is pinned by T1's exhaustive cases. + +## 10. Estimate, speed, acceptance + +| file | net lines (counted: Python, not tests or docs) | +|---|---| +| `src_python/tin_engine/mcp/schema.py` (type table, `DerivedCommand`, model, argv, `Tool`) | about 85 | +| `src_python/tin_engine/mcp/server.py` (`RunResult`, child runner with progress, heartbeat and kill, the sidecar names and their removal, the increasing-progress sender, result shaping, errors, semaphore, `serve()`) | about 150 | +| `src_python/tin_engine/mcp/__init__.py`, `src_python/tin_engine/__main__.py` | 3 | +| `src_python/tin_engine/cli.py`: the `mcp` command with `--jobs`, the lazy import and its refusal | about 20 | +| `pyproject.toml`, `.github/workflows/main.yaml`, `README.md`, `test_layering.py` rows | 0 (not counted) | +| total | about 260 (250 before review round 1; the sidecar naming and the progress sender add about 10); with the +39 % worst case of increment 33, about 360: one PR, under 700 | + +Speed of existing commands: expected unchanged. The increment adds a +command to the Typer app (registration at import, microseconds), a +`__main__.py` nobody else runs, and a new package nothing else imports; no +refine or mesh code is touched, so no `@perf` acceptance run is due. The +probe's `rasputin mesh --tolerance 1` on the benchmark tile, 0.72 s, is the +same CLI the server will run. + +## 11. Ideas file + +Taken up: "MCP server over the rasputin commands" (this increment, as ruled). +Overlapped: "Intent layer over rasputin" (its MCP server step is this one, +built from the CLI rather than from a `MeshIntent` schema; its OpenAPI +export and provider benchmark are not taken up); "Dual-use applications" and +"Feature-adaptive vertical tolerance" (the direction, intent in and terrain +out, is what the tools are for; nothing of them is built here); "Release +hygiene with priority" (README's "Connect an agent" section lands here, the +rest stays there); "Product review: readable code docs" (tool descriptions +inherit the docstrings it will rewrite). Handed back under *Ideas* in the +handback: phase progress lines from `mesh`, an OpenAPI export from +`schema.py`, Streamable HTTP for hosted agents, Tasks when the SDK has them, +a jobs cap by memory estimate rather than count. + +## 12. As built (recorded by the main session from the handbacks) + +Ola picked the MCP server on 2026-10-11 ("Add an MCP server to the ideas +file after GeoPackage") and ruled the dependency on 2026-10-12 ("Rule the +mcp SDK: yes, use it"); the design's nine questions were decided on their +defaults while he was away (section 10). Design `b990e38c`, three review +rounds (Review section), the last two closed on the reviewer's exact +wording in the recording commit. + +**Red** `ee5b5d52` (`@tester`): 62 tests, every one red on the missing +module `tin_engine.mcp`, the missing `tin_engine.__main__`, the stdio +server dying at import, or `rasputin mcp` not being a subcommand; the +existing 7,233 tests pass (full suite 9 min 56 s, coverage 98.6 %). +Files: `tests/python/test_mcp_schema.py` (T1-T3, 26), +`test_mcp_server.py` (T4-T6, T8, T10 in process, 22), `test_mcp_stdio.py` +(T7-T9, 5), `test_mcp_cli.py` (T11, 2, run without the SDK too), +`test_layering.py` rows (T12, 7), helpers `mcp_driver.py`, +`mcp_fake_child.py`, `mcp_stdio_launcher.py`. The three SDK suites skip +when `mcp` is absent. Pins beyond the design, asserted: `derive(app)` drops +the command named `mcp`; `DerivedCommand.tool_name`, `.model`, `.argv()`, +`.tool()`; `argv[0]` is the hyphenated CLI name and argv follows the +command's parameter order; an Enum default is its value, a Path default its +string; `build_server(app, *, jobs=1, child=)`; the +`HEARTBEAT_SECONDS` module constant; error results carry `stderr` in +`structured_content`; the success text block is the structured JSON +verbatim. Departures from the design: `__main__.py` carries an +`if __name__ == "__main__"` guard (the layering test imports it); the +Kartverket triangle count is a window (410k to 520k) with the tolerance read +from the record, since the count is not a record field; "within 0.05 s" on +progress became exact two-sided bounds. + +**Green** `851592ba` (`@developer`): `src_python/tin_engine/mcp/` +(`__init__.py`, `schema.py` 98 lines, `server.py` 204), `__main__.py` (5), +`cli.py` (15: the `mcp` subcommand and the `_destination` hint), +`pyproject.toml` (the `mcp` extra, `mcp>=2.3`, pulled by `dev`). No test +file touched. All 62 red tests green; full `pytest tests/python/` 7,292 +passed, 17 skipped, coverage 98.43 % (`mcp/schema.py` 100 %, +`mcp/server.py` 91 %); Kartverket through the tool 464,290 triangles in +0.77 s; `ruff check`, `ruff format --check`, `mypy` (strict, 72 files), +`check_prohibited_deps.py` (fails naming rasterio when planted in the `mcp` +extra, restored) and `check_detria_boundary.py` clean. No C++ touched. +Departures, for the code review to judge: (1) `cli.py`'s `_destination` +gains a `hint` (default `--out`) used as `param_hint` on its three +refusals, and the `--out-edges`, `--stats`, `--record` and `--out-dir` +callers pass theirs: section 6.4 and T6 say an `out` outside `out_parent` +is "refused naming `--out`", but at `b4a81c0d` the CLI said +`Invalid value: X is outside the permitted parent Y`, naming no option; +refusals now read `Invalid value for --out: ...` (Ola's question: keep? +default keep, it is what the design and the plain-output rule ask). (2) +`RunResult` lives in `schema.py`, not `server.py` as section 5 sites it: +`tool()` needs it and the layering rows forbid `schema` importing +`server`; `server` re-exports it. (3) Section 6.1's four `main.yaml` +install-line edits are not made: `tests/python/test_ci_changes.py` pins +those strings; `dev` pulls `mcp>=2.3` instead (the main session's default). + +**Size:** 322 net production lines (`python3 tools/count_loc.py b4a81c0d +HEAD`): schema 98, server 204, cli 15, `__main__` 5; the design estimated +260 with a worst case of 360. Under 700. + +**README** (`@architect`, code review round 1, blocker 1): section 6.8's +"Connect an agent" section landed in `README.md` between "Output and +viewing" and "How it is built" (`git log --oneline -1 -- README.md` names +the commit), human reader first as Ola ruled on 2026-10-11, about 240 words +of prose plus the Claude Desktop and Codex configuration blocks; every claim +in it was checked against `server.py`, `schema.py`, the `mcp` subcommand and +`pyproject.toml` at `730162fe`, the derived tool list in the worktree venv +(eight tools, `fetch_stations` and `station_catchments` with underscores), +and the two clients' own configuration pages. + +**Decided while Ola was away (defaults):** the nine design questions +(section 10); the `dev` extra pulls `mcp` so the `python` CI job runs the +suite; the `_destination` hint kept pending Ola's word. + +**After the master merge** (`030fd32b`, @developer): master `c317c7e4` +brought increment 26b and the ruling record; `ROADMAP.md` (row 36 against +row 37) and `tests/python/test_layering.py` (26b's rows against this +branch's, one shared `cli` row importing both `land_cover_fractions` and +`mcp.server`) were resolved by hand; `_core` rebuilt in `build-wt/`; all +gates green, ctest 1031/1031, 328 net lines (the five are `01025786`'s +help-text escape, round 2). One test broke: `test_mcp_schema.py` pinned +`mesh` at 34 parameters, now 36 with `--land-cover` and +`--land-cover-year` (83 across the eight tools; the design's 81 held at +`b4a81c0d`). `@tester` (`d8a1729d`) dropped the per-tool counts rather than +update them: the same test's equality check, schema properties against the +command's click parameters, implies the count, and the pin only detected +options added elsewhere; the eight tool names stay pinned. This changes the +design's T3 wording (section 9) from counts to names plus equality; a probe +dropping `land_cover` from the derived `mesh` schema turns the surviving +check red. The help-text escape (`01025786`): one constant `INSTALL_MCP`, +escaped to `\[mcp]` only when `app.rich_markup_mode` is set, so both +renderings read `pip install "rasputin[mcp]"`. + +**Lessons for the next retrospective:** a design claim about existing CLI +wording is checked by running the command before a test pins it (T6's +"naming `--out`"); no worktree had a current `_core` build, so `@tester` +copied one from a merged worktree with identical C++ rather than build +beside `@perf`. A pinned snapshot of another module's shape (a CLI's option +count) in a derivation suite is a resolved value where a rule belongs; pin +the rule and the identity set, never the count. + +## Review + +Design review round 1, 2026-10-12, @reviewer (`b4a81c0d..b990e38c`, 0 production lines, estimate 250 with worst case 350): CHANGES REQUESTED, three blockers. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@b990e38c:382-386 and :450 say every output goes through `.part` and a rename; that holds for the GeoPackage only, `.vtk` and `.ply` are written with `target.write_bytes` (`src_python/tin_engine/cli.py@b4a81c0d:1241`) and `_writing` (:2121) wraps OSError, it does not rename; restate that a killed `.vtk`/`.ply` run can leave a truncated file under the asked name. (2) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@b990e38c:354-357 and :255: with `out_parent` set, the server's temporary `--record`/`--stats` path is refused by `_destination` (`src_python/tin_engine/cli.py@b4a81c0d:442-445` via :1360), so every such `mesh` call exits 2; say where the temporary file goes (inside `out_parent`, or beside `out`) and add the case to T4-T6. (3) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@b990e38c:364-367 and :396-398: the heartbeat and "waiting" notifications carry no `progress` value, which the specification says MUST increase per notification; define it (elapsed seconds) and have T9 assert the increase across lines and heartbeats. Prose fixes for the same pass: :148 "1 of 81" is 2 (`draw` and `mesh` both have `snap_spacing`); :54 the timeout-reset quote is on the specification's Cancellation page, not Progress; :296 the `python` CI job (`.github/workflows/main.yaml:261`) also installs `.[dev]`; :97 the server list now says 100. Suggestions: state that the server process writes nothing but JSON-RPC to its own stdout; T11 also drops `tin_engine.mcp.server` from `sys.modules`. Verified: every probe figure against the saved output; SDK 2.3.0 facts in a fresh venv (low-level `Server`, `jsonschema` only in the client session, no progress token on the direct dispatcher, no server-side Tasks, 13 non-Windows dependencies); the specification quotes, the legacy grep, check_citations, and the prohibited-deps scan of extras. Ideas for Ola's file: write every mesh output through `.part` plus rename in `cli.py`; `destructive_hint=False` while `--out` overwrites an existing mesh deserves a second look. + +Design review round 2, 2026-10-12, @reviewer (`b9a692de..cb534b36`, 0 production lines): CHANGES REQUESTED, one blocker, applied in this recording commit as the reviewer specified: /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@cb534b36:363-370 started the child without saying it inherits the server's working directory (no `cwd=`), which the read-back and `unlink` of a relative `out`'s sidecar depend on, and no test pinned a relative `out`; one sentence added there, and T6 gains a relative-`out` case under `monkeypatch.chdir`. The reviewer's idea applied with it: T6 says `outputs` is exactly the `.vtk`, so the echoed sidecar path (`src_python/tin_engine/cli.py@b4a81c0d:1247`) is excluded. Verified: round 1's fixes against `cli.py@b4a81c0d` (`write_bytes` :1241, `_write_geopackage` :1250-1289 with `os.replace` :1289, `_destination` :438-445, the sidecar accepted exactly when `out` is, absolute, relative or through a symlinked parent); the specification quotes on the 2026-07-28 Progress and Cancellation pages; `main.yaml` installs `.[dev]` in four jobs; check_citations exit 0. Lesson: an assumption named only in a handback is not in the design; the file must carry it. + +Design review round 3, 2026-10-12, @reviewer (`cb534b36..37f811c1`, 0 production lines): CHANGES REQUESTED, one blocker, applied in this recording commit as specified: /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@37f811c1:612 cited `cli.py` line 1247 at `b4a81c0d` without its directory path, so `tools/check_citations.py` exited 1 on the branch; now `src_python/tin_engine/cli.py@b4a81c0d:1247`, the record-path echo, so the claim holds. The cwd sentence and both T6 insertions read in place; round 2's citation resolves. The design proceeds to the red suite on this path fix without a fourth round, as the recording-commit practice allows (retrospective 2026-10-12, P1). Lesson for the main session: run `python3 tools/check_citations.py` from inside the worktree after each recording commit; the main checkout's copy judges the main checkout's tree. + +**Code review, round 1, 2026-10-12.** Range `b4a81c0d..3859b138`, 322 net production lines (schema 98, server 204, cli 15, `__main__` 5; the overrun over 260 is `server.py`'s shielded clean-up, heartbeat and resource links; under 700), @reviewer: CHANGES REQUESTED, four blockers. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@3859b138:508-509 and :665 say the README gains a "Connect an agent" section; `README.md` is unchanged and section 12 records no departure; the section is written by `@architect`. (2) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/src_python/tin_engine/mcp/server.py@3859b138:147-148 joins every non-Usage stderr line into `error`, where the design (:291-292) says the `Error:` line's words; a `fetch` failing mid-run prints progress lines first (`src_python/tin_engine/cli.py@3859b138:1543`, then :1549), so `error` reads "exit 1: 1,000 of 2,000 bytes Error: ..."; fix: keep the `Error:` lines when any exist, else all; `@tester` adds the T5 case first, `@developer` fixes. (3) Two citations this branch shifted, pinned at `b4a81c0d` in this recording commit: /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-mcp-server.md@3859b138:500 (`test_layering.py` 143-149, the function now at 146-152) and /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/research/generic-harness.md@3859b138:192 (`pyproject.toml` 108, now 115). (4) Red-step prose remains in /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/tests/python/mcp_driver.py@3859b138:10-12 and /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/tests/python/mcp_stdio_launcher.py@3859b138:33-34 ("before the increment is built"); `@tester` rewords to the present. Verified: 159 tests green in the worktree venv (Kartverket 464,290 triangles, 0.83 s); sections 5 to 7 against the code (schema from click parameters with `extra="forbid"`, `mcp` dropped; child argv, three environment variables, no `cwd=`; sidecars beside `out`, unlinked shielded; progress strictly increasing with the heartbeat; cancel kills; the semaphore; `file://` links; no print); the I/O boundary (the server imports only `installed_version` and `schema`); prohibited-deps and citation gates exit 0; the green commit touched no test; the three departures sound (each `_destination` hint matches its caller's option; `RunResult` re-exported; `dev` pulling `mcp` reaches every `.[dev]` CI job). Question for Ola: keep the `_destination` hint, default keep. Ideas (for Ola's file): `schema._annotation` drops a `min=0`/`max=0` bound (`if v`); await the cancelled heartbeat task so no tick races the result. Lesson: a design that names a README section needs the as-built record to say it landed or why not. + +**Code review, round 2, 2026-10-12.** Range `3859b138..dad373fe`, 323 net production lines, @reviewer: CHANGES REQUESTED, two blockers, both prose. (1) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/src_python/tin_engine/cli.py@dad373fe:278: the help text `pip install "rasputin[mcp]"` renders as `pip install "rasputin"` under Typer's default Rich markup, which eats `[mcp]` as a tag (measured with `python -m tin_engine mcp --help`; correct only with `TYPER_USE_RICH=0`); `@developer` escapes the bracket for Rich and re-renders, in the same step as the master merge. (2) /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/README.md@dad373fe:135-136 said the server reports every line the command prints; only stderr lines become progress (`src_python/tin_engine/mcp/server.py@dad373fe:116-119`), stdout returns in the result; fixed in this recording commit to "every line the command prints to stderr". Verified: round 1's four fixes against their sources; the two T5 cases match `_refusal` as fixed; 57 MCP tests pass (Kartverket 464,290 triangles, 0.83 s); every README claim against the code and both clients' pages; no red-step scaffolding; check_citations exit 0. Idea (for Ola's file): `python -m tin_engine` usage lines read `python -m tin_engine.rasputin` from the argv[0] rename; `app(prog_name="rasputin")` would fix it. Lesson: help text is a rendered claim; check it as the default renderer shows it. + +**Code review, round 3, 2026-10-12.** Range `dad373fe..0615aff7`, 328 net production lines by `count_loc.py origin/master HEAD` (323 at `dad373fe`; the five are the help-text escape), @reviewer: APPROVED, no blockers. Verified: both help renderings print `pip install "rasputin[mcp]"` (Rich default and `TYPER_USE_RICH=0`), and `mcp` is absent from the tool list so the escaped copy reaches no description; the merge `030fd32b` diffed against each parent carries only the other side's rows in `test_layering.py` and `ROADMAP.md`, the `cli` row matches `cli.py` (`land_cover_fractions` at module level, `mcp.server` inside `mcp()`), `_core` rebuilt; `d8a1729d`: set equality implies the count, dropping `land_cover` from the derived `mesh` schema turns it False, 36 for `mesh` and 83 in all measured, the eight names pinned; section 12's paragraph and the five pinned "81" match the commits; check_citations exit 0; MCP suites plus layering 126 passed; ruff clean. Noted: the ctest 1031/1031 figure is in @developer's handback, not in `030fd32b`'s message; master's C++ is unchanged by this branch. Idea applied: ROADMAP row 37 says 328 lines. diff --git a/docs/increments/37-probes/mcp_probe.py b/docs/increments/37-probes/mcp_probe.py new file mode 100644 index 00000000..4d05c52b --- /dev/null +++ b/docs/increments/37-probes/mcp_probe.py @@ -0,0 +1,474 @@ +"""Increment 37 probe: what the `mcp` SDK makes of the rasputin commands, and +what each execution route costs. Design evidence, not a test: run it by hand, +`python mcp_probe.py`, in a venv that has mcp>=2.3, imagecodecs and +`tin_engine` importable with a current `_core`. `--serve` makes this file the +stdio server the probe spawns, so the stdio round trip goes through the SDK's +own client. + +The server here is the design in miniature: the SDK's low-level `Server`, one +tool per Typer command, each tool's input schema derived from the command's +click parameters (one source of truth) with `additionalProperties: false`, +arguments validated by the derived Pydantic model, the command run as a child +process with `TYPER_USE_RICH=0`, and the exit code, output lines, record and +plain-word refusal returned as data. + +Parts (each prints lines starting with its letter): + A the derived schemas, listed back through the SDK's client; which of the + parameters needed a note; + B error mapping: a refused command (exit 2), an argument of the wrong type, + an unknown argument, an unknown tool; + C a tool awaiting a child process, cancelled by the client's timeout, in + process and over stdio: does the child die? + D two concurrent calls; + E timings: interpreter start, `import tin_engine.cli`, `rasputin version` + and the 1 m benchmark `mesh` as subprocesses; in-process `version`; + in-process and stdio tool round trips; the mesh through the tool, with + progress notifications counted. + +The production server (increment 37) must not copy two shortcuts taken here: +the handler reads and unlinks temp files with blocking `Path` calls (ASYNC240), +and the measurement lines run past 100 columns so each stays one grep-able line. +""" +# ruff: noqa: E501, ASYNC240 + +from __future__ import annotations + +import asyncio +import json +import os +import statistics +import subprocess +import sys +import tempfile +import time +from pathlib import Path +from typing import Any, Literal + +import anyio +import click +import mcp_types as types +import typer +from mcp.client import Client +from mcp.client.stdio import StdioServerParameters +from mcp.server.context import ServerRequestContext +from mcp.server.lowlevel import Server +from mcp.server.stdio import stdio_server +from mcp.shared.exceptions import MCPError +from pydantic import BaseModel, ConfigDict, Field, ValidationError, create_model + +HERE = Path(__file__).resolve() +REPO = HERE.parents[3] +DEM = REPO / "tests/fixtures/dem_archive/7908_3_10m_z33.tif" +CLI = [sys.executable, "-c", "from tin_engine.cli import app; app()"] +CHILD_ENV = {**os.environ, "TYPER_USE_RICH": "0", "NO_COLOR": "1", "TERM": "dumb"} + +SCALAR = {"str": str, "text": str, "int": int, "integer": int, "float": float, "bool": bool, + "boolean": bool, "path": str} # fmt: skip + + +def py_type(t: click.ParamType) -> tuple[Any, dict[str, Any], str | None]: + """A click ParamType as an annotation, Field constraints, and a note or None.""" + if "int" in t.name and hasattr(t, "min") and hasattr(t, "max"): # click's and Typer's IntRange + return int, {"ge": t.min, "le": t.max}, None + if isinstance(t, click.Choice): + return Literal[tuple(t.choices)], {}, None + inner = getattr(t, "types", None) + if inner is not None: # Typer's and click's tuple types both carry .types + parts = [py_type(x) for x in inner] + return tuple[tuple(p[0] for p in parts)], {}, next((p[2] for p in parts if p[2]), None) + if t.name in SCALAR: + return SCALAR[t.name], {}, None + return str, {}, f"unmapped click type {t.name!r}" + + +class Derived: + """A Typer command as a tool: its argument model and the way back to argv.""" + + def __init__(self, cmd: click.Command, tool_name: str) -> None: + self.cmd, self.tool_name, self.notes = cmd, tool_name, [] + fields: dict[str, Any] = {} + for p in cmd.params: + if p.name in ("help", "install_completion", "show_completion"): + continue + typ, constraints, note = py_type(p.type) + if note: + self.notes.append(f"{p.name}: {note}") + if getattr(p, "callback", None) is not None: + self.notes.append(f"{p.name}: callback {p.callback.__name__} stays in the child") + if p.multiple: + typ = list[typ] # type: ignore[valid-type] + help_text = getattr(p, "help", None) or "" + if p.required: + fields[p.name] = (typ, Field(description=help_text, **constraints)) + else: + default = p.default() if callable(p.default) else p.default + if default is None or (p.multiple and default == ()): + default, typ = None, typ | None # type: ignore[operator] + fields[p.name] = (typ, Field(default, description=help_text, **constraints)) + self.model: type[BaseModel] = create_model( + f"{tool_name}_arguments", __config__=ConfigDict(extra="forbid"), **fields + ) + + def tool(self) -> types.Tool: + schema = self.model.model_json_schema() + schema.pop("title", None) + return types.Tool( + name=self.tool_name, + description=(self.cmd.help or "").split("\n\n")[0], + input_schema=schema, + ) + + def argv(self, args: BaseModel) -> list[str]: + """The command line the arguments mean; only the keys the caller set.""" + argv = [self.cmd.name or self.tool_name] + given = args.model_fields_set + for p in self.cmd.params: + if p.name not in given: + continue + value = getattr(args, p.name) + if p.param_type_name == "argument": # Typer 0.27's TyperArgument is no click.Argument + argv.extend(str(v) for v in (value if p.nargs != 1 else [value])) + continue + opt = p.opts[0] + if getattr(p, "is_flag", False): + if value: + argv.append(opt) + elif p.secondary_opts: + argv.append(p.secondary_opts[0]) + continue + items = value if p.multiple else [value] + for item in items: + argv.append(opt) + argv.extend(str(v) for v in (item if isinstance(item, tuple) else [item])) + return argv + + +def derive_all() -> dict[str, Derived]: + from tin_engine.cli import app + + root = typer.main.get_command(app) + return {n.replace("-", "_"): Derived(c, n.replace("-", "_")) for n, c in root.commands.items()} + + +def error_result(exit_code: int | None, message: str) -> types.CallToolResult: + return types.CallToolResult( + content=[types.TextContent(type="text", text=message)], + structured_content={"exit_code": exit_code, "error": message}, + is_error=True, + ) + + +def refusal_words(stderr: str) -> str: + """Click's own `Error:` line(s), without the usage reminder.""" + lines = [ln for ln in stderr.splitlines() if ln and not ln.startswith(("Usage:", "Try '"))] + return " ".join(lines).removeprefix("Error: ").strip() + + +CHILDREN: list[asyncio.subprocess.Process] = [] + + +async def run_child( + ctx: ServerRequestContext[Any], argv: list[str] +) -> tuple[int, list[str], list[str]]: + """Run `rasputin argv` as a child; a stderr line is a progress tick; die with the caller.""" + proc = await asyncio.create_subprocess_exec( + *CLI, *argv, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, env=CHILD_ENV + ) + CHILDREN.append(proc) + token = (ctx.meta or {}).get("progress_token") + err_lines: list[str] = [] + + async def read_stderr() -> None: + assert proc.stderr is not None + async for raw in proc.stderr: + err_lines.append(raw.decode(errors="replace").rstrip()) + if token is not None: + await ctx.session.send_progress_notification( + token, len(err_lines), None, err_lines[-1][:200] + ) + + try: + assert proc.stdout is not None + async with asyncio.TaskGroup() as tg: + tg.create_task(read_stderr()) + out_task = tg.create_task(proc.stdout.read()) + await proc.wait() + finally: + if proc.returncode is None: + proc.kill() + await proc.wait() + return proc.returncode or 0, out_task.result().decode(errors="replace").splitlines(), err_lines + + +def build_server() -> tuple[Server[Any], dict[str, Derived]]: + derived = derive_all() + + async def list_tools(ctx: Any, params: Any) -> types.ListToolsResult: + slow = types.Tool( + name="slow", + description="Await a child `sleep`; kill it if cancelled.", + input_schema={ + "type": "object", + "properties": {"seconds": {"type": "number"}, "pid_file": {"type": "string"}}, + "required": ["seconds", "pid_file"], + "additionalProperties": False, + }, + ) + return types.ListToolsResult(tools=[d.tool() for d in derived.values()] + [slow]) + + async def call_tool( + ctx: ServerRequestContext[Any], params: types.CallToolRequestParams + ) -> types.CallToolResult: + args = params.arguments or {} + if params.name == "slow": + proc = await asyncio.create_subprocess_exec("sleep", str(args["seconds"])) + Path(args["pid_file"]).write_text(str(proc.pid)) + try: + await proc.wait() + finally: + if proc.returncode is None: + proc.kill() + await proc.wait() + return types.CallToolResult(content=[], structured_content={"exit_code": proc.returncode}) + d = derived.get(params.name) + if d is None: + raise MCPError(code=types.INVALID_PARAMS, message=f"Unknown tool: {params.name}") + try: + model = d.model.model_validate(args) + except ValidationError as exc: + words = "; ".join(f"{'.'.join(map(str, e['loc']))}: {e['msg']}" for e in exc.errors()) + return error_result(None, words) + argv = d.argv(model) + record_tmp: Path | None = None + if params.name == "mesh" and "record_path" not in model.model_fields_set: + record_tmp = Path(tempfile.mkstemp(suffix=".json")[1]) + argv += ["--record", str(record_tmp)] + t0 = time.perf_counter() + code, out, err = await run_child(ctx, argv) + result: dict[str, Any] = { + "exit_code": code, + "seconds": round(time.perf_counter() - t0, 3), + "outputs": [ln for ln in out if ln and Path(ln).exists()] if params.name != "palette" else [], + "stdout": out, + "stderr": err, + } + if record_tmp is not None: + if record_tmp.exists() and record_tmp.stat().st_size: + result["record"] = json.loads(record_tmp.read_text()) + same = record_tmp.resolve() # the CLI prints the resolved path (/private/var on macOS) + result["stdout"] = [ln for ln in out if Path(ln).resolve() != same] + result["outputs"] = [p for p in result["outputs"] if Path(p).resolve() != same] + record_tmp.unlink(missing_ok=True) + if code != 0: + return error_result(code, f"exit {code}: {refusal_words(os.linesep.join(err))}") + return types.CallToolResult( + content=[types.TextContent(type="text", text=json.dumps(result)[:4000])], + structured_content=result, + ) + + server: Server[Any] = Server("rasputin-probe", on_list_tools=list_tools, on_call_tool=call_tool) + return server, derived + + +async def amedian(fn: Any, n: int) -> tuple[float, float]: + walls = [] + for _ in range(n): + t0 = time.perf_counter() + await fn() + walls.append(time.perf_counter() - t0) + return statistics.median(walls), max(walls) + + +def median_wall(fn: Any, n: int) -> tuple[float, float]: + walls = [] + for _ in range(n): + t0 = time.perf_counter() + fn() + walls.append(time.perf_counter() - t0) + return statistics.median(walls), max(walls) + + +def alive(pid_file: Path) -> str: + pid = int(pid_file.read_text()) + try: + os.kill(pid, 0) + except ProcessLookupError: + return f"pid {pid} dead" + done = subprocess.run(["ps", "-o", "stat=", "-p", str(pid)], capture_output=True, text=True) + return f"pid {pid} ALIVE (ps stat {done.stdout.strip()!r})" if done.stdout.strip() else f"pid {pid} dead" + + +async def cancel_check(client: Client, label: str, tmp: Path) -> None: + pid_file = tmp / f"{label}.pid" + t0 = time.perf_counter() + try: + await client.call_tool("slow", {"seconds": 30, "pid_file": str(pid_file)}, read_timeout_seconds=1.0) + print(f"C {label}: timeout did not raise") + except Exception as exc: + print(f"C {label}: client raised {type(exc).__name__} after {time.perf_counter() - t0:.2f} s") + await anyio.sleep(0.5) + print(f"C {label}: child sleep {alive(pid_file)}") + + +async def probe(tmp: Path) -> None: + server, derived = build_server() + async with Client(server) as client: + listed = await client.list_tools() + tools = {t.name: t for t in listed.tools} + print(f"A tools listed: {sorted(tools)}; protocol {client.protocol_version}") + print(f"A palette input_schema:\n{json.dumps(tools['palette'].input_schema, indent=1)}") + mesh = tools["mesh"].input_schema + print( + f"A mesh: {len(mesh['properties'])} properties, required {mesh.get('required')}, " + f"additionalProperties {mesh.get('additionalProperties')}, {len(json.dumps(mesh))} bytes" + ) + for key in ("bbox", "tolerance_near", "binary", "features", "snap_spacing"): + print(f"A mesh.{key}: {json.dumps(mesh['properties'][key])}") + print(f"A fetch.connections: {json.dumps(tools['fetch'].input_schema['properties']['connections'])}") + print(f"A all schemas: {sum(len(json.dumps(t.input_schema)) for t in tools.values())} bytes") + for name, d in derived.items(): + for note in d.notes: + print(f"A note {name}: {note}") + + r = await client.call_tool("mesh", {"dem": [str(DEM)], "bbox": [1, 2, 3, 4]}) + print(f"B refused (no out): is_error={r.is_error} structured={r.structured_content}") + r = await client.call_tool("mesh", {"out": str(tmp / "x.vtk"), "dem": ["/nonexistent.tif"]}) + print(f"B refused (bad dem): is_error={r.is_error} structured={r.structured_content}") + r = await client.call_tool("palette", {"name": 3}) + print(f"B wrong type: is_error={r.is_error} text={r.content[0].text!r}") + r = await client.call_tool("palette", {"name": "corine", "bogus": 1}) + print(f"B unknown argument: is_error={r.is_error} text={r.content[0].text!r}") + r = await client.call_tool("mesh", {"out": str(tmp / "x.vtk"), "tolerance": "ten"}) + print(f"B wrong type nested: is_error={r.is_error} text={r.content[0].text!r}") + try: + r = await client.call_tool("nosuch", {}) + print(f"B unknown tool: is_error={r.is_error} text={r.content[0].text!r}") + except Exception as exc: + print(f"B unknown tool: client raised {type(exc).__name__}: {exc}") + r = await client.call_tool("palette", {"name": "corine"}) + sc = r.structured_content or {} + print( + f"B palette corine: is_error={r.is_error}, exit {sc.get('exit_code')}, " + f"{len(sc.get('stdout', []))} stdout lines, {sc.get('seconds')} s, error {sc.get('error')!r}" + ) + + await cancel_check(client, "in-process", tmp) + + t0 = time.perf_counter() + a, b = await asyncio.gather(client.call_tool("version", {}), client.call_tool("version", {})) + print( + f"D two concurrent version calls: {time.perf_counter() - t0:.2f} s, exits " + f"{(a.structured_content or {}).get('exit_code')}, {(b.structured_content or {}).get('exit_code')}" + ) + + med, mx = await amedian(lambda: client.call_tool("version", {}), 5) + print(f"E in-process round trip, version tool: median {med:.3f} s max {mx:.3f}") + + ticks: list[str] = [] + + async def on_progress(progress: float, total: float | None, message: str | None) -> None: + ticks.append(message or "") + + out = tmp / "tool.vtk" + r = await client.call_tool( + "mesh", {"dem": [str(DEM)], "tolerance": 1.0, "out": str(out)}, progress_callback=on_progress + ) + sc = r.structured_content or {} + print( + f"E mesh through the tool: is_error={r.is_error}, child {sc.get('seconds')} s, " + f"outputs {sc.get('outputs')}, record keys {len(sc.get('record', {}))}, " + f"stderr lines {len(sc.get('stderr', []))}, progress ticks {len(ticks)}, " + f"text block {len(r.content[0].text)} chars" + ) + print(f"E mesh stderr lines: {sc.get('stderr')}") + print(f"E mesh record sample: {json.dumps(dict(list(sc.get('record', {}).items())[:6]))}") + + t0 = time.perf_counter() + params = StdioServerParameters(command=sys.executable, args=[str(HERE), "--serve"]) + async with Client(params) as client: + connect = time.perf_counter() - t0 + listed = await client.list_tools() + med, mx = await amedian(lambda: client.call_tool("version", {}), 5) + print( + f"E stdio: connect+initialize {connect:.3f} s, {len(listed.tools)} tools, " + f"version round trip median {med:.3f} s max {mx:.3f}" + ) + await cancel_check(client, "stdio", tmp) + ticks = [] + r = await client.call_tool( + "mesh", + {"dem": [str(DEM)], "tolerance": 1.0, "out": str(tmp / "stdio.vtk")}, + progress_callback=on_progress, + ) + sc = r.structured_content or {} + print( + f"E stdio mesh through the tool: is_error={r.is_error}, child {sc.get('seconds')} s, " + f"progress ticks {len(ticks)} {ticks[:2]}, outputs {len(sc.get('outputs', []))}, " + f"record keys {len(sc.get('record', {}))}" + ) + + +def subprocess_timings(tmp: Path) -> None: + def sub(args: list[str]) -> Any: + return lambda: subprocess.run(args, capture_output=True, check=True) + + med, mx = median_wall(sub([sys.executable, "-c", "pass"]), 5) + print(f"E python -c pass: median {med:.3f} s max {mx:.3f}") + med, mx = median_wall(sub([sys.executable, "-c", "import tin_engine.cli"]), 5) + print(f"E import tin_engine.cli: median {med:.3f} s max {mx:.3f}") + med, mx = median_wall(sub([sys.executable, "-c", "import mcp.server.lowlevel, mcp.client"]), 5) + print(f"E import mcp (server+client): median {med:.3f} s max {mx:.3f}") + med, mx = median_wall(sub([*CLI, "version"]), 5) + print(f"E rasputin version (subprocess): median {med:.3f} s max {mx:.3f}") + + from typer.testing import CliRunner + + from tin_engine.cli import app + + runner = CliRunner() + med, mx = median_wall(lambda: runner.invoke(app, ["version"]), 5) + print(f"E rasputin version (in-process CliRunner): median {med:.4f} s max {mx:.4f}") + + out, rec, walls = tmp / "m.vtk", tmp / "r.json", [] + for _ in range(3): + out.unlink(missing_ok=True) + t0 = time.perf_counter() + done = subprocess.run( + [*CLI, "mesh", "--dem", str(DEM), "--tolerance", "1", "--out", str(out), "--record", str(rec)], + capture_output=True, text=True, env=CHILD_ENV, + ) # fmt: skip + walls.append(time.perf_counter() - t0) + if done.returncode != 0: + print(f"E mesh FAILED: {done.stderr[-400:]}") + return + record = json.loads(rec.read_text()) + print( + f"E rasputin mesh 7908_3 --tolerance 1 (subprocess): median {statistics.median(walls):.2f} s " + f"of {[round(w, 2) for w in walls]}; stdout {done.stdout.splitlines()}; " + f"stderr {len(done.stderr.splitlines())} lines; record {rec.stat().st_size} bytes, " + f"{len(record)} keys; mesh {out.stat().st_size / 1e6:.1f} MB" + ) + + +def main() -> None: + if "--serve" in sys.argv: + server, _ = build_server() + + async def serve() -> None: + async with stdio_server() as (read, write): + await server.run(read, write, server.create_initialization_options()) + + anyio.run(serve) + return + import importlib.metadata as m + + print(f"probe {HERE}\npython {sys.version.split()[0]}, mcp {m.version('mcp')}, typer " + f"{m.version('typer')}, pydantic {m.version('pydantic')}") # fmt: skip + print(subprocess.run(["pmset", "-g", "batt"], capture_output=True, text=True).stdout.splitlines()[0]) + with tempfile.TemporaryDirectory() as tmp: + anyio.run(probe, Path(tmp)) + subprocess_timings(Path(tmp)) + + +if __name__ == "__main__": + main() diff --git a/docs/increments/37-probes/mcp_probe_output.txt b/docs/increments/37-probes/mcp_probe_output.txt new file mode 100644 index 00000000..edd23605 --- /dev/null +++ b/docs/increments/37-probes/mcp_probe_output.txt @@ -0,0 +1,66 @@ +probe /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/docs/increments/37-probes/mcp_probe.py +python 3.14.7, mcp 2.3.0, typer 0.27.3, pydantic 2.14.1 +Now drawing from 'AC Power' +A tools listed: ['catchment', 'draw', 'fetch', 'fetch_stations', 'mesh', 'palette', 'slow', 'station_catchments', 'version']; protocol 2026-07-28 +A palette input_schema: +{ + "type": "object", + "additionalProperties": false, + "properties": { + "name": { + "description": "The palette: corine.", + "title": "Name", + "type": "string" + }, + "out": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "description": "Write the preset here, not to stdout.", + "title": "Out" + } + }, + "required": [ + "name" + ] +} +A mesh: 34 properties, required ['out'], additionalProperties False, 8269 bytes +A mesh.bbox: {"anyOf": [{"maxItems": 4, "minItems": 4, "prefixItems": [{"type": "number"}, {"type": "number"}, {"type": "number"}, {"type": "number"}], "type": "array"}, {"type": "null"}], "default": null, "description": "With --dem, mesh only XMIN YMIN XMAX YMAX in the DEM's CRS, snapped outward to its nodes.", "title": "Bbox"} +A mesh.tolerance_near: {"anyOf": [{"maxItems": 2, "minItems": 2, "prefixItems": [{"type": "string"}, {"type": "number"}], "type": "array"}, {"type": "null"}], "default": null, "description": "Hold the mesh to N metres on these lines (.geojson, .gpkg or .gml). Needs --tolerance-ramp.", "title": "Tolerance Near"} +A mesh.binary: {"default": true, "description": "Binary files, small and fast; --ascii writes text you can read.", "title": "Binary", "type": "boolean"} +A mesh.features: {"anyOf": [{"items": {"type": "string"}, "type": "array"}, {"type": "null"}], "default": null, "description": "With --domain, polygons and lines as constraints (.geojson, .json, .gpkg or .gml), clipped to the domain, each edge carrying its class map's bits. Repeatable: each --features is one source, paired by position with the Nth of --features-crs, --features-layer and --features-map.", "title": "Features"} +A mesh.snap_spacing: {"default": 0.001, "description": "Snap grid spacing for the noder, in the input's units.", "title": "Snap Spacing", "type": "number"} +A fetch.connections: {"default": 8, "description": "", "minimum": 1, "title": "Connections", "type": "integer"} +A all schemas: 16575 bytes +A note draw: snap_spacing: callback _snap_spacing stays in the child +A note mesh: snap_spacing: callback _snap_spacing stays in the child +B refused (no out): is_error=True structured={'exit_code': None, 'error': 'out: Field required'} +B refused (bad dem): is_error=True structured={'exit_code': 2, 'error': 'exit 2: Invalid value for --dem: cannot read /nonexistent.tif: No such file or directory'} +B wrong type: is_error=True text='name: Input should be a valid string' +B unknown argument: is_error=True text='bogus: Extra inputs are not permitted' +B wrong type nested: is_error=True text='tolerance: Input should be a valid number, unable to parse string as a number' +B unknown tool: client raised MCPError: Unknown tool: nosuch +B palette corine: is_error=False, exit 0, 239 stdout lines, 0.26 s, error None +C in-process: client raised MCPError after 1.00 s +C in-process: child sleep pid 41644 dead +D two concurrent version calls: 0.29 s, exits 0, 0 +E in-process round trip, version tool: median 0.261 s max 0.265 +E mesh through the tool: is_error=False, child 0.715 s, outputs ['/private/var/folders/h2/yycsx2z16_d181syfgb0947m0000gn/T/tmp063_dw0p/tool.vtk'], record keys 38, stderr lines 1, progress ticks 0, text block 1958 chars +E mesh stderr lines: ['464290 triangles. Every DEM node inside the mesh is within 1 m of it (largest difference 0.99998 m). 199 vertices on NoData cells were removed with their triangles.'] +E mesh record sample: {"rasputin_version": "unknown (rasputin is not installed)", "command": "-c mesh --out /var/folders/h2/yycsx2z16_d181syfgb0947m0000gn/T/tmp063_dw0p/tool.vtk --dem /Users/skavhaug/projects/rasputin/.claude/worktrees/mcp-server/tests/fixtures/dem_archive/7908_3_10m_z33.tif --tolerance 1.0 --record /var/folders/h2/yycsx2z16_d181syfgb0947m0000gn/T/tmpyypcqotq.json", "dem_grid": "5051 columns x 5051 rows, 10 m apart", "dem_vertical_unit": "metres", "start_mesh": "every 40th DEM node", "start_min_angle_deg": 25.0} +E stdio: connect+initialize 0.465 s, 9 tools, version round trip median 0.264 s max 0.277 +C stdio: client raised MCPError after 1.00 s +C stdio: child sleep pid 41935 dead +E stdio mesh through the tool: is_error=False, child 0.719 s, progress ticks 1 ['464290 triangles. Every DEM node inside the mesh is within 1 m of it (largest difference 0.99998 m). 199 vertices on NoData cells were removed with their triangles.'], outputs 1, record keys 38 +E python -c pass: median 0.019 s max 0.020 +E import tin_engine.cli: median 0.247 s max 0.249 +E import mcp (server+client): median 0.399 s max 0.401 +E rasputin version (subprocess): median 0.262 s max 0.266 +E rasputin version (in-process CliRunner): median 0.0107 s max 0.0120 +E rasputin mesh 7908_3 --tolerance 1 (subprocess): median 0.72 s of [0.72, 0.72, 0.72]; stdout ['/private/var/folders/h2/yycsx2z16_d181syfgb0947m0000gn/T/tmp063_dw0p/m.vtk', '/private/var/folders/h2/yycsx2z16_d181syfgb0947m0000gn/T/tmp063_dw0p/r.json']; stderr 1 lines; record 1583 bytes, 38 keys; mesh 16.9 MB diff --git a/docs/research/generic-harness.md b/docs/research/generic-harness.md index 72bf78ea..6063da79 100644 --- a/docs/research/generic-harness.md +++ b/docs/research/generic-harness.md @@ -189,7 +189,7 @@ point at it. | Prohibited dependencies | `CLAUDE.md@3245958:23-26`, checked by `tools/check_prohibited_deps.py` | `.claude/agents/architect.md@3245958:30` ("No-GDAL and No-CGAL"); `.claude/skills/computational-geometry/SKILL.md@3245958:24` ("Do not use CGAL"); `.claude/skills/geospatial-data-formats/SKILL.md@3245958:10-11` (GDAL, OGR, Fiona) and `.claude/skills/geospatial-data-formats/SKILL.md@3245958:14` ("Never `rasterio`"), plus "Zero-GDAL" in its description and summary (`.claude/skills/geospatial-data-formats/SKILL.md@3245958:3`, `.claude/skills/geospatial-data-formats/SKILL.md@3245958:8`). Pointers, *ok*: `.claude/agents/orchestrator.md@3245958:56`, `testing.md@3245958:351` | Each becomes "see `CLAUDE.md` §2". The geospatial skill keeps its how-to (tifffile, shapely, pyproj) | | Size ceiling | `CLAUDE.md@3245958:15-22` | `docs/PRINCIPLES.md@3245958:220` (its heading states "700"). Pointers, *ok*: `.claude/agents/developer.md@3245958:30`, `.claude/agents/reviewer.md@3245958:16`, `.claude/agents/tester.md@3245958:68`, `.claude/agents/orchestrator.md@3245958:25`, and the three skills' "Change Limit" lines (redundant but harmless) | Rename E4 without the number | | Reconcile LOC against the estimate | `.claude/agents/reviewer.md@3245958:43-45` (check 3) | `.claude/agents/reviewer.md@3245958:16-20` (the same file, again); `.claude/agents/developer.md@3245958:30-33`; `.claude/agents/orchestrator.md@3245958:29-30` | Keep check 3; the others point at it | -| Coverage floor | `testing.md@3245958:279`, checked by `pyproject.toml:108` (`--cov-fail-under=85`) | `CLAUDE.md@3245958:9` ("85%"); `.claude/agents/tester.md@3245958:13-14` and `.claude/agents/tester.md@3245958:20-23` (twice in one file); `testing.md@3245958:18` and `testing.md@3245958:283-288` | State it once at `testing.md@3245958:279`; drop the number everywhere else | +| Coverage floor | `testing.md@3245958:279`, checked by `pyproject.toml@b4a81c0d:108` (`--cov-fail-under=85`) | `CLAUDE.md@3245958:9` ("85%"); `.claude/agents/tester.md@3245958:13-14` and `.claude/agents/tester.md@3245958:20-23` (twice in one file); `testing.md@3245958:18` and `testing.md@3245958:283-288` | State it once at `testing.md@3245958:279`; drop the number everywhere else | | Approval boundary | `.claude/REQUIRED-READING.md@3245958:100-116` | `docs/PRINCIPLES.md@3245958:200-206` (E1 restates the list of acts); the message in `guard_push.py`. Pointers, *ok*: `CLAUDE.md@3245958:43-45`, `.claude/agents/orchestrator.md@3245958:38-43` | E1 becomes an index entry (§4) | | `@reviewer` before a push | `.claude/REQUIRED-READING.md@3245958:118-126` | `docs/PRINCIPLES.md@3245958:208-211` (E2) | Index entry | | Claims discipline | `.claude/REQUIRED-READING.md@3245958:59-80` | `docs/PRINCIPLES.md@3245958:24-57` and `docs/PRINCIPLES.md@3245958:91-108` (A1-A4, B1, B2 restate it in different words) | Index entries | diff --git a/pyproject.toml b/pyproject.toml index f1234793..6061aff9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,7 +59,14 @@ codecs = ["imagecodecs>=2024.1"] # the wheels are 100-140 MB. It carries no GDAL reader (13-bundled-mesh.md, # "Dependency facts"). CI installs it in its own step. viewer = ["vtk>=9.3"] +# `rasputin mcp` (increment 37): the official Model Context Protocol SDK, MIT, +# 13 pure-Python dependencies, none on CLAUDE.md section 2's list. Without it +# the command refuses with an install line; every other command is unchanged. +# `dev` pulls it so the suite's MCP tests run wherever the suite does (CI +# installs `.[dev]`); without it they skip. +mcp = ["mcp>=2.3"] dev = [ + "mcp>=2.3", "pytest>=8.0", "pytest-asyncio>=0.23", "pytest-cov>=4.1", diff --git a/src_python/tin_engine/__main__.py b/src_python/tin_engine/__main__.py new file mode 100644 index 00000000..8519445a --- /dev/null +++ b/src_python/tin_engine/__main__.py @@ -0,0 +1,12 @@ +"""``python -m tin_engine``: the CLI, as the MCP server's child runs it (increment 37, section 6.4). + +``sys.argv[0]`` is renamed so the run record's ``command`` field reads ``rasputin mesh ...``. +""" + +import sys + +from tin_engine.cli import app + +if __name__ == "__main__": + sys.argv[0] = "rasputin" + app() diff --git a/src_python/tin_engine/cli.py b/src_python/tin_engine/cli.py index 33fdfd9b..3cd69964 100644 --- a/src_python/tin_engine/cli.py +++ b/src_python/tin_engine/cli.py @@ -268,6 +268,33 @@ def version() -> None: typer.echo(f"bounds checks: {bounds_checks()}") +#: The install line for the ``mcp`` extra. Rich help reads ``[mcp]`` as a markup +#: tag and drops it, so the help text escapes the bracket exactly when the app +#: renders help through Rich (``rich_markup_mode`` is None without Rich, where +#: Click's plain help would print the backslash). +INSTALL_MCP = 'pip install "rasputin[mcp]"' +_INSTALL_MCP_HELP = INSTALL_MCP.replace("[", r"\[") if app.rich_markup_mode else INSTALL_MCP + + +@app.command( + help="Serve the commands to an agent over the Model Context Protocol, on stdin and stdout." + "\n\nOne tool per command, each call run as ``python -m tin_engine`` (increment 37). " + f"Needs the ``mcp`` extra: ``{_INSTALL_MCP_HELP}``." +) +def mcp( + jobs: Annotated[ + int, typer.Option("--jobs", min=1, help="Tool calls run at once; one beyond this waits.") + ] = 1, +) -> None: + """Serve the commands to an agent over the Model Context Protocol (``--help`` says how).""" + try: + from tin_engine.mcp import server + except ImportError: + typer.echo(f"Error: the mcp extra is not installed; run {INSTALL_MCP}", err=True) + raise typer.Exit(2) from None + asyncio.run(server.serve(app, jobs)) + + @dataclass(frozen=True, slots=True) class Attempt: """What the engine made of a fixture, and what to draw it from. @@ -419,8 +446,9 @@ def _snap_spacing(value: float) -> float: return value -def _destination(out: Path | None, out_parent: Path | None, name: str) -> Path: - """Where to write, having refused every path this process should not touch. +def _destination(out: Path | None, out_parent: Path | None, name: str, hint: str = "--out") -> Path: + """Where to write, having refused every path this process should not touch; a + refusal names the option, ``hint``. A hostile boundary, per the python skill. ``resolve()`` first, so that ``..`` cannot smuggle a write out of an explicitly permitted parent; refuse @@ -438,14 +466,16 @@ def _destination(out: Path | None, out_parent: Path | None, name: str) -> Path: # so a second run cannot overwrite the picture still open in a browser. return Path(tempfile.mkdtemp(prefix="rasputin-")) / f"{name}.svg" if out.is_symlink(): - raise typer.BadParameter(f"{out} is a symlink; refusing to follow it") + raise typer.BadParameter(f"{out} is a symlink; refusing to follow it", param_hint=hint) target = out.resolve() if out_parent is not None: permitted = out_parent.resolve() if not target.is_relative_to(permitted): - raise typer.BadParameter(f"{target} is outside the permitted parent {permitted}") + raise typer.BadParameter( + f"{target} is outside the permitted parent {permitted}", param_hint=hint + ) if not target.parent.is_dir(): - raise typer.BadParameter(f"{target.parent} is not an existing directory") + raise typer.BadParameter(f"{target.parent} is not an existing directory", param_hint=hint) return target @@ -1225,7 +1255,7 @@ def mesh( # writes succeed, the second overwrites the first, the command echoes # two paths and exits 0 -- the caller has lost the surface they asked # for and nothing said so. - constraints = _destination(out_edges, out_parent, label) + constraints = _destination(out_edges, out_parent, label, "--out-edges") if constraints == surface: raise typer.BadParameter( f"--out and --out-edges both resolve to {surface}; " @@ -1489,7 +1519,7 @@ def _report_target( checked before any mesh file is written.""" if stats is None or stats == "-": return None - target = _destination(Path(stats), out_parent, label) + target = _destination(Path(stats), out_parent, label, "--stats") if target in meshes: raise typer.BadParameter( f"resolves to {target}; the report would overwrite the mesh", param_hint="--stats" @@ -1506,7 +1536,7 @@ def _record_target( return None if record == "-": raise typer.BadParameter("standard output is --stats -'s", param_hint="--record") - target = _destination(Path(record), out_parent, label) + target = _destination(Path(record), out_parent, label, "--record") if target in taken: raise typer.BadParameter( f"resolves to {target}, which this run also writes", param_hint="--record" @@ -2649,7 +2679,7 @@ def station_catchments( math.isfinite(outline_tolerance) and outline_tolerance >= 0.0 ): raise typer.BadParameter("must be finite and at least 0", param_hint="--outline-tolerance") - target = _destination(out_dir, out_parent, out_dir.name) + target = _destination(out_dir, out_parent, out_dir.name, "--out-dir") try: station_list, stations_crs = read_stations(stations) except (OSError, ValueError) as exc: diff --git a/src_python/tin_engine/mcp/__init__.py b/src_python/tin_engine/mcp/__init__.py new file mode 100644 index 00000000..9eb13ed2 --- /dev/null +++ b/src_python/tin_engine/mcp/__init__.py @@ -0,0 +1,5 @@ +"""The rasputin commands served over the Model Context Protocol (increment 37). + +`schema` derives one tool per Typer command; `server` runs each call as a +child process. Both need the optional ``mcp`` extra. +""" diff --git a/src_python/tin_engine/mcp/schema.py b/src_python/tin_engine/mcp/schema.py new file mode 100644 index 00000000..d2da580b --- /dev/null +++ b/src_python/tin_engine/mcp/schema.py @@ -0,0 +1,138 @@ +"""Each Typer command as an MCP tool: its argument model, its JSON schema, the way back to argv. + +Increment 37 (`docs/increments/37-mcp-server.md`, sections 5 and 6.3). Pure: +a Typer app in, `DerivedCommand`s out; validated arguments in, argv out. No +I/O, no async, nothing of `tin_engine` imported, so the derivation is tested +on a toy app without the kernel. The click parameters are the one statement +of the CLI's surface, so a new option is a new property with no change here; +`additionalProperties: false` because the CLI refuses an unknown option too. +""" + +from __future__ import annotations + +import enum +from dataclasses import dataclass +from pathlib import PurePath +from typing import Any, Literal + +import mcp_types as types +import typer +from pydantic import BaseModel, ConfigDict, Field, create_model + +#: The command that is the server itself: not a tool (section 6.8). +SERVER_COMMAND = "mcp" +#: click's type names as annotations (section 5); a path is a string on the server's machine. +SCALARS: dict[str, type] = {"str": str, "int": int, "float": float, "boolean": bool, "path": str} +#: Section 5's annotations by tool name; every tool is non-destructive. +READ_ONLY = frozenset({"version"}) +OPEN_WORLD = frozenset({"fetch", "fetch_stations"}) + + +class RunResult(BaseModel): + """What a tool call returns when the command exits 0 (section 5's table).""" + + command: str + exit_code: int + seconds: float + outputs: list[str] + stdout: list[str] + stderr: list[str] + record: dict[str, Any] | None = None + stats: str | None = None + + +def _annotation(param_type: Any) -> tuple[Any, dict[str, Any]]: + """A click parameter type as an annotation and the `Field` constraints it implies.""" + if hasattr(param_type, "choices"): + return Literal[tuple(param_type.choices)], {} + if hasattr(param_type, "types"): # a tuple type: fixed length, one type per place + return tuple[tuple(_annotation(t)[0] for t in param_type.types)], {} # type: ignore[misc] + bounds = {"ge": getattr(param_type, "min", None), "le": getattr(param_type, "max", None)} + return SCALARS.get(param_type.name.split()[0], str), {k: v for k, v in bounds.items() if v} + + +def _default(param: Any) -> Any: + value = param.default() if callable(param.default) else param.default + if isinstance(value, enum.Enum): + return value.value + return str(value) if isinstance(value, PurePath) else value + + +def _fields(command: Any) -> dict[str, Any]: + """`create_model` field definitions, one per click parameter, in the command's order.""" + fields: dict[str, Any] = {} + for param in command.params: + annotation, constraints = _annotation(param.type) + if param.multiple: + annotation = list[annotation] # type: ignore[valid-type] + if param.required: + fields[param.name] = (annotation, Field(description=param.help or None, **constraints)) + continue + default = _default(param) + if default is None: + annotation = annotation | None + info = Field(default, description=param.help or None, **constraints) + fields[param.name] = (annotation, info) + return fields + + +def _words(value: Any) -> list[str]: + return [str(v) for v in value] if isinstance(value, tuple) else [str(value)] + + +@dataclass(frozen=True) +class DerivedCommand: + """A click command, its tool name, and the Pydantic model of its arguments.""" + + command: Any + tool_name: str + model: type[BaseModel] + + def tool(self) -> types.Tool: + schema = self.model.model_json_schema() + schema.pop("title", None) + read_only = self.tool_name in READ_ONLY + return types.Tool( + name=self.tool_name, + description=" ".join((self.command.help or "").split("\n\n")[0].split()), + input_schema=schema, + output_schema=RunResult.model_json_schema(), + annotations=types.ToolAnnotations( + read_only_hint=read_only, + destructive_hint=False, + idempotent_hint=read_only, + open_world_hint=self.tool_name in OPEN_WORLD, + ), + ) + + def argv(self, arguments: BaseModel) -> list[str]: + """The command line the arguments mean: only the keys the caller set, in the CLI's order.""" + argv = [str(self.command.name)] + for param in self.command.params: + value = getattr(arguments, param.name) + if param.name not in arguments.model_fields_set or value is None: + continue + if param.param_type_name == "argument": + argv.extend(_words(value)) + elif getattr(param, "is_flag", False): + argv.extend([param.opts[0]] if value else param.secondary_opts[:1]) + else: + for item in value if param.multiple else [value]: + argv.extend([max(param.opts, key=len), *_words(item)]) + return argv + + +def derive(app: typer.Typer) -> list[DerivedCommand]: + """One `DerivedCommand` per command of ``app``, in its order, the server's own excluded.""" + root: Any = typer.main.get_command(app) + return [ + DerivedCommand( + command, + name.replace("-", "_"), + create_model( + f"{name}_arguments", __config__=ConfigDict(extra="forbid"), **_fields(command) + ), + ) + for name, command in root.commands.items() + if name != SERVER_COMMAND + ] diff --git a/src_python/tin_engine/mcp/server.py b/src_python/tin_engine/mcp/server.py new file mode 100644 index 00000000..7640abff --- /dev/null +++ b/src_python/tin_engine/mcp/server.py @@ -0,0 +1,269 @@ +"""``rasputin mcp``: the commands served over the Model Context Protocol, each call a child process. + +Increment 37 (`docs/increments/37-mcp-server.md`, sections 4 to 7). The +package's only I/O and only async code: the child ``python -m tin_engine +``, its stdout and stderr read as data, the record and stats sidecars +beside ``out`` (removed after the read), progress notifications carrying +elapsed seconds, a jobs cap, and the kill on cancel. Nothing here imports +`_core`: the kernel runs in the child, behind the CLI, which keeps every rule +it has. This process's stdout is the wire, so nothing here prints. +""" + +from __future__ import annotations + +import asyncio +import json +import os +import secrets +import shlex +import signal +import sys +import time +from collections.abc import AsyncIterator, Sequence +from contextlib import asynccontextmanager +from pathlib import Path +from typing import Any + +import anyio +import mcp_types as types +import typer +from anyio import Path as AsyncPath +from mcp.server.context import ServerRequestContext +from mcp.server.lowlevel import Server +from mcp.server.stdio import stdio_server +from mcp.shared.exceptions import MCPError +from pydantic import BaseModel, ValidationError + +from tin_engine import installed_version +from tin_engine.mcp.schema import DerivedCommand, RunResult, derive + +__all__ = ["HEARTBEAT_SECONDS", "RunResult", "build_server", "serve"] + +#: Seconds between heartbeats while the child prints nothing (section 6.5); read at every tick. +HEARTBEAT_SECONDS = 5.0 +#: The production child: the same interpreter and installed `tin_engine` as the server. +CHILD = (sys.executable, "-m", "tin_engine") +#: Typer's plain refusals and no colour in the child (section 2). +CHILD_ENV = {"TYPER_USE_RICH": "0", "NO_COLOR": "1", "TERM": "dumb"} +MIME_TYPES = { + ".gpkg": "application/geopackage+sqlite3", + ".vtk": "application/octet-stream", + ".ply": "application/octet-stream", + ".geojson": "application/geo+json", + ".json": "application/json", + ".md": "text/markdown", +} +#: Shown to the model at initialize (section 6.8). +INSTRUCTIONS = """\ +Paths are on the machine this server runs on, as for the rasputin command line. +mesh writes a mesh file at out and returns its run record; the mesh itself never travels here. +Write a .gpkg to inspect a mesh with GIS tools; .vtk is for ParaView. +A catalogue DEM source must be fetched first: run fetch over the domain, then mesh. +A refusal's words say what to change; the exit code is the command's. +A cancelled mesh may leave an unreadable .vtk or .ply at out; rerun before reading it.""" + + +class _Progress: + """One call's clock and sender: each notification carries elapsed seconds, increasing.""" + + def __init__(self, ctx: ServerRequestContext[Any]) -> None: + self.began = time.monotonic() + self.token = ctx.meta.get("progress_token") if ctx.meta else None + self.session = ctx.session + self.last = -1.0 + self.lock = anyio.Lock() + + def elapsed(self) -> float: + return time.monotonic() - self.began + + async def send(self, message: str) -> None: + if self.token is None: + return + async with self.lock: # one sender per call, so values leave in order + self.last = max(round(self.elapsed(), 3), self.last + 0.001) + await self.session.send_progress_notification(self.token, self.last, None, message) + + @asynccontextmanager + async def heartbeat(self, phrase: str) -> AsyncIterator[None]: + """``", N s"`` every `HEARTBEAT_SECONDS` while the body runs; nothing without a + progress token.""" + + async def tick() -> None: + while True: + await asyncio.sleep(HEARTBEAT_SECONDS) + await self.send(f"{phrase}, {self.elapsed():.1f} s") + + task = asyncio.create_task(tick()) if self.token is not None else None + try: + yield + finally: + if task is not None: + task.cancel() + + +async def _run_child(argv: Sequence[str], progress: _Progress) -> tuple[int, list[str], list[str]]: + """Run ``argv`` to the end, or kill it when cancelled; each stderr line is a notification.""" + process = await asyncio.create_subprocess_exec( + *argv, + stdin=asyncio.subprocess.DEVNULL, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + env={**os.environ, **CHILD_ENV}, + ) + assert process.stdout is not None and process.stderr is not None # both are PIPE + stderr: list[str] = [] + + async def read_stderr(pipe: asyncio.StreamReader) -> None: + async for raw in pipe: + stderr.append(raw.decode(errors="replace").rstrip("\r\n")) + await progress.send(stderr[-1]) + + try: + stdout, _ = await asyncio.gather(process.stdout.read(), read_stderr(process.stderr)) + code = await process.wait() + finally: + if process.returncode is None: + process.kill() + with anyio.CancelScope(shield=True): + await process.wait() + return code, stdout.decode(errors="replace").splitlines(), stderr + + +def _error( + exit_code: int | None, error: str, stderr: list[str] | None = None +) -> types.CallToolResult: + body: dict[str, Any] = {"exit_code": exit_code, "error": error} + if stderr is not None: + body["stderr"] = stderr + return types.CallToolResult( + content=[types.TextContent(type="text", text=error)], structured_content=body, is_error=True + ) + + +def _refusal(code: int, stderr: list[str]) -> str: + """The CLI's own words: its ``Error:`` line(s) without the usage reminder (section 5).""" + if code < 0: + return f"killed by signal {-code} ({signal.Signals(-code).name})" + words = [line for line in stderr if line and not line.startswith(("Usage:", "Try '"))] + errors = [line.removeprefix("Error: ") for line in words if line.startswith("Error: ")] + return f"exit {code}: {' '.join(errors or words)}" + + +async def _read(path: Path | None) -> str | None: + return await AsyncPath(path).read_text() if path and await AsyncPath(path).is_file() else None + + +async def _outputs(stdout: list[str], hidden: list[Path]) -> list[str]: + """The stdout lines naming something that exists after the run, less the sidecars.""" + taken = [await AsyncPath(p).resolve() for p in hidden] + outputs = [] + for line in stdout: + if ( + line.strip() + and await AsyncPath(line).exists() + and (await AsyncPath(line).resolve() not in taken) + ): + outputs.append(line) + return outputs + + +async def _call( + tool: DerivedCommand, arguments: BaseModel, prefix: Sequence[str], progress: _Progress +) -> types.CallToolResult: + """One validated call: the sidecars, the child, the shaped result (sections 5 and 6.4).""" + argv = tool.argv(arguments) + sidecars: list[Path] = [] + record_file = stats_file = None + if tool.tool_name == "mesh": + out, token = Path(getattr(arguments, "out")), secrets.token_hex(4) # noqa: B009 + if "record_path" in arguments.model_fields_set: + record_file = Path(getattr(arguments, "record_path")) # noqa: B009 + else: + record_file = out.parent / f".{out.name}.record-{token}.json" + argv += ["--record", str(record_file)] + sidecars.append(record_file) + if getattr(arguments, "stats", None) == "-": + stats_file = out.parent / f".{out.name}.stats-{token}.md" + argv[argv.index("--stats") + 1] = str(stats_file) + sidecars.append(stats_file) + try: + began = time.monotonic() + code, stdout, stderr = await _run_child([*prefix, *argv], progress) + seconds = round(time.monotonic() - began, 3) + if code != 0: + return _error(code, _refusal(code, stderr), stderr) + record = await _read(record_file) + result = RunResult( + command=shlex.join(["rasputin", *argv]), + exit_code=code, + seconds=seconds, + outputs=await _outputs(stdout, sidecars), + stdout=stdout, + stderr=stderr, + record=json.loads(record) if record else None, + stats=await _read(stats_file), + ) + links = [ + types.ResourceLink( + type="resource_link", + name=Path(p).name, + uri=(await AsyncPath(p).resolve()).as_uri(), + mime_type=MIME_TYPES.get(Path(p).suffix), + ) + for p in result.outputs + ] + finally: + with anyio.CancelScope(shield=True): + for sidecar in sidecars: + await AsyncPath(sidecar).unlink(missing_ok=True) + body = result.model_dump() + return types.CallToolResult( + content=[types.TextContent(type="text", text=json.dumps(body)), *links], + structured_content=body, + ) + + +def build_server(app: typer.Typer, *, jobs: int = 1, child: Sequence[str] = CHILD) -> Server[Any]: + """The server over ``app``'s commands; ``child`` is the argv prefix a command is run with.""" + tools = {d.tool_name: d for d in derive(app)} + semaphore, prefix = anyio.Semaphore(jobs), list(child) + + async def list_tools(ctx: ServerRequestContext[Any], params: Any) -> types.ListToolsResult: + return types.ListToolsResult(tools=[d.tool() for d in tools.values()]) + + async def call_tool( + ctx: ServerRequestContext[Any], params: types.CallToolRequestParams + ) -> types.CallToolResult: + tool = tools.get(params.name) + if tool is None: + raise MCPError(types.INVALID_PARAMS, f"Unknown tool: {params.name}") + try: + arguments = tool.model.model_validate(params.arguments or {}) + except ValidationError as exc: + return _error(None, "; ".join(f"{e['loc'][0]}: {e['msg']}" for e in exc.errors())) + progress = _Progress(ctx) + try: + async with progress.heartbeat("waiting for a running job"): + await semaphore.acquire() + try: + async with progress.heartbeat("running"): + return await _call(tool, arguments, prefix, progress) + finally: + semaphore.release() + except Exception as exc: # the server's own fault, in words (section 5) + return _error(None, f"{type(exc).__name__}: {exc}") + + return Server( + "rasputin", + version=installed_version(), + instructions=INSTRUCTIONS, + on_list_tools=list_tools, + on_call_tool=call_tool, + ) + + +async def serve(app: typer.Typer, jobs: int = 1) -> None: + """``rasputin mcp``: the server on this process's stdin and stdout (section 6.8).""" + server = build_server(app, jobs=jobs) + async with stdio_server() as (read, write): + await server.run(read, write, server.create_initialization_options()) diff --git a/tests/python/mcp_driver.py b/tests/python/mcp_driver.py new file mode 100644 index 00000000..fc6ea057 --- /dev/null +++ b/tests/python/mcp_driver.py @@ -0,0 +1,186 @@ +"""One way to drive the rasputin MCP server in the tests (increment 37). + +`docs/increments/37-mcp-server.md`, section 9. Test support only; nothing in +`src_python` uses it. The suites take these from here rather than from each +other, so no test module is a helper library for another. + +The server under test is `tin_engine.mcp.server.build_server(app, *, jobs, +child)`, where ``child`` is the argv prefix the server runs a command with +(production: ``[sys.executable, "-m", "tin_engine"]``). + +In process, the SDK's `Client(server)` connects without a transport; it +carries no progress token (section 2, measured), so progress is asserted over +stdio only, through `stdio_server`, which spawns `mcp_stdio_launcher`. +""" + +from __future__ import annotations + +import json +import os +import re +import subprocess +import sys +import time +from collections.abc import Sequence +from dataclasses import dataclass, field +from itertools import pairwise +from pathlib import Path +from typing import Any + +import anyio +from mcp.client import Client +from mcp.client.stdio import StdioServerParameters +from mcp_types import CallToolResult + +HERE = Path(__file__).resolve().parent +FAKE_CHILD = HERE / "mcp_fake_child.py" +LAUNCHER = HERE / "mcp_stdio_launcher.py" + +#: The tool names in the CLI's order (section 5), read from the app at test +#: time so the list and the CLI cannot drift; the server's own command is not +#: a tool. +SERVER_COMMAND = "mcp" + +#: The sidecar names the server chooses beside ``out`` (section 6.4): a dot, +#: the mesh's file name, ``record-`` or ``stats-``, eight hex digits. +RECORD_SIDECAR = re.compile(r"^\.(?P.+)\.record-[0-9a-f]{8}\.json$") +STATS_SIDECAR = re.compile(r"^\.(?P.+)\.stats-[0-9a-f]{8}\.md$") + +#: Every notification of a call carries elapsed seconds, never a count +#: (section 6.5); the heartbeat says so in words. +RUNNING = re.compile(r"^running, [0-9.]+ s$") +WAITING = re.compile(r"^waiting for a running job, [0-9.]+ s$") + + +def tool_names(app: Any) -> list[str]: + """The expected tool list: the CLI's commands, in its order, ``-`` as ``_``.""" + import typer + + commands = typer.main.get_command(app).commands + return [name.replace("-", "_") for name in commands if name != SERVER_COMMAND] + + +def fake_child(tmp_path: Path, **spec: Any) -> list[str]: + """A ``child`` prefix running `mcp_fake_child` with ``spec`` (its docstring lists the keys).""" + path = tmp_path / f"fake-child-{len(list(tmp_path.glob('fake-child-*.json')))}.json" + path.write_text(json.dumps(spec)) + return [sys.executable, str(FAKE_CHILD), str(path)] + + +def stdio_server(tmp_path: Path, **config: Any) -> StdioServerParameters: + """The launcher as a stdio server (`mcp_stdio_launcher`: ``child``, ``jobs``, ``heartbeat``). + + The environment is passed whole: the SDK's default is a short allow-list, + and the server's child must find the same interpreter and data as the test. + """ + path = tmp_path / "launcher.json" + path.write_text(json.dumps(config)) + return StdioServerParameters( + command=sys.executable, args=[str(LAUNCHER), str(path)], env=dict(os.environ) + ) + + +def production_server() -> StdioServerParameters: + """``python -m tin_engine mcp``: the entry an agent's configuration names (section 6.8).""" + return StdioServerParameters( + command=sys.executable, args=["-m", "tin_engine", "mcp"], env=dict(os.environ) + ) + + +async def call(server: Any, name: str, arguments: dict[str, Any], **options: Any) -> CallToolResult: + """One tool call on a fresh connection; ``server`` is a `Server` or stdio parameters.""" + async with Client(server) as client: + return await client.call_tool(name, arguments, **options) + + +def structured(result: CallToolResult) -> dict[str, Any]: + assert isinstance(result.structured_content, dict), result + return result.structured_content + + +def text_block(result: CallToolResult) -> str: + """The one text block of a result.""" + texts = [b.text for b in result.content if b.type == "text"] + assert len(texts) == 1, result.content + return texts[0] + + +def resource_links(result: CallToolResult) -> list[tuple[str, str | None]]: + """``(uri, mime_type)`` of each ``resource_link`` block, in order.""" + return [(b.uri, b.mime_type) for b in result.content if b.type == "resource_link"] + + +def dead(pid: int) -> bool: + """Gone, or a zombie nobody has reaped yet (a kill without a wait).""" + try: + os.kill(pid, 0) + except ProcessLookupError: + return True + done = subprocess.run(["ps", "-o", "stat=", "-p", str(pid)], capture_output=True, text=True) + state = done.stdout.strip() + return state == "" or state.startswith("Z") + + +def pid_of(pid_file: Path) -> int: + return int(pid_file.read_text()) + + +async def wait_until_dead(pid_file: Path, within: float) -> None: + """The pid in ``pid_file`` is dead within ``within`` seconds of now, polled at 50 ms.""" + pid = pid_of(pid_file) + deadline = time.monotonic() + within + while not dead(pid): + assert time.monotonic() < deadline, f"child {pid} still alive {within} s after the cancel" + await anyio.sleep(0.05) + + +@dataclass +class Progress: + """Every progress notification of one call, with the client's own clock at receipt.""" + + began: float = field(default_factory=time.monotonic) + ticks: list[tuple[float, float | None, str | None, float]] = field(default_factory=list) + + async def __call__(self, progress: float, total: float | None, message: str | None) -> None: + self.ticks.append((progress, total, message, time.monotonic() - self.began)) + + @property + def values(self) -> list[float]: + return [t[0] for t in self.ticks] + + @property + def messages(self) -> list[str | None]: + return [t[2] for t in self.ticks] + + def assert_strictly_increasing(self) -> None: + values = self.values + assert all(b > a for a, b in pairwise(values)), values + + def assert_elapsed_seconds(self) -> None: + """Each value is seconds since the handler began: never more than the client's own + elapsed time at receipt (the handler began after the call was sent), and ``total`` + is absent (the run's length is unknown).""" + for progress, total, _message, received in self.ticks: + assert total is None, self.ticks + assert 0.0 <= progress <= received, (progress, received, self.ticks) + + +def option_value(argv: list[str], flag: str) -> str: + """The value after ``flag``, which appears exactly once in ``argv``.""" + assert argv.count(flag) == 1, (flag, argv) + return argv[argv.index(flag) + 1] + + +def intervals(path: Path) -> list[tuple[float, float]]: + """`mcp_fake_child`'s ``interval_file`` lines as ``(start, end)``, by start.""" + rows = [json.loads(line) for line in path.read_text().splitlines()] + return sorted((r["start"], r["end"]) for r in rows) + + +def overlap(a: tuple[float, float], b: tuple[float, float]) -> bool: + return a[0] < b[1] and b[0] < a[1] + + +def only_files(directory: Path, names: Sequence[str]) -> None: + """``directory`` holds exactly ``names`` (so no sidecar was left behind).""" + assert sorted(p.name for p in directory.iterdir()) == sorted(names), list(directory.iterdir()) diff --git a/tests/python/mcp_fake_child.py b/tests/python/mcp_fake_child.py new file mode 100644 index 00000000..7f0c14f1 --- /dev/null +++ b/tests/python/mcp_fake_child.py @@ -0,0 +1,69 @@ +"""A stand-in for ``python -m tin_engine`` under the MCP server's child runner. + +Increment 37 (`docs/increments/37-mcp-server.md`, sections 6.4 and 9). The +server runs ``[*child, , ...]``; a test gives it +``[sys.executable, this file, SPEC.json]`` as ``child``, so ``sys.argv[2:]`` is +exactly the argv the server built from the tool arguments. The spec, written +by `mcp_driver.fake_child`, says what to do, in this order: + +``argv_file`` + write ``sys.argv[2:]`` here as JSON (T4: what the server passed). +``pid_file`` + write this process's pid (T8: is it dead after a cancel?). +``create_record`` + write ``{}`` to the ``--record`` path found in argv (T5: a sidecar the + child did create, which the server must still remove). +``stdout`` + lines to print to stdout, at once. +``stderr`` + ``[seconds, line]`` pairs: sleep, then print the line to stderr, flushed + (T9: each line is one progress notification). +``sleep`` + seconds to sleep before exiting (T8: a run to cancel; T10: a run to queue). +``interval_file`` + append ``{"start": t, "end": t}`` wall-clock seconds, one JSON line (T10: + did two runs overlap?). +``signal`` + end by sending this signal number to itself (T5: ``killed by signal``). +``exit`` + the exit code, default 0. + +Test support only; nothing in `src_python` runs it. +""" + +from __future__ import annotations + +import json +import os +import sys +import time +from pathlib import Path + + +def main() -> None: + spec = json.loads(Path(sys.argv[1]).read_text()) + argv = sys.argv[2:] + started = time.time() + if "argv_file" in spec: + Path(spec["argv_file"]).write_text(json.dumps(argv)) + if "pid_file" in spec: + Path(spec["pid_file"]).write_text(str(os.getpid())) + if spec.get("create_record") and "--record" in argv: + Path(argv[argv.index("--record") + 1]).write_text("{}") + for line in spec.get("stdout", []): + print(line, flush=True) + for seconds, line in spec.get("stderr", []): + time.sleep(seconds) + print(line, file=sys.stderr, flush=True) + time.sleep(spec.get("sleep", 0.0)) + if "interval_file" in spec: + with open(spec["interval_file"], "a") as f: + f.write(json.dumps({"start": started, "end": time.time()}) + "\n") + if "signal" in spec: + os.kill(os.getpid(), int(spec["signal"])) + time.sleep(5.0) # never reached: the signal ends the process + sys.exit(int(spec.get("exit", 0))) + + +if __name__ == "__main__": + main() diff --git a/tests/python/mcp_stdio_launcher.py b/tests/python/mcp_stdio_launcher.py new file mode 100644 index 00000000..15dff627 --- /dev/null +++ b/tests/python/mcp_stdio_launcher.py @@ -0,0 +1,47 @@ +"""The rasputin MCP server over stdio with a test's child runner and heartbeat. + +Increment 37 (`docs/increments/37-mcp-server.md`, sections 6.5, 6.6 and 9). +``rasputin mcp`` runs the real CLI as its child with a 5 s heartbeat; T8 to +T10 over stdio need the server with `mcp_fake_child` as the child and, for +T9, a heartbeat of 0.1 s. This file is the server process the SDK's client +spawns for those tests: ``python mcp_stdio_launcher.py CONFIG.json``, where +the config, written by `mcp_driver.stdio_server`, holds + +``child`` + the argv prefix the server runs (`mcp_driver.fake_child`'s), or absent + for the production default; +``jobs`` + the concurrent-jobs cap (default 1); +``heartbeat`` + seconds between heartbeats, or absent for the module's constant. + +Everything else is the production server: `build_server` on the real Typer +app, run by the SDK's own ``stdio_server``. Test support only. +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +import anyio +from mcp.server.stdio import stdio_server + +from tin_engine.cli import app +from tin_engine.mcp import server as server_module + + +async def main(config: dict[str, object]) -> None: + if "heartbeat" in config: + server_module.HEARTBEAT_SECONDS = float(config["heartbeat"]) + options: dict[str, object] = {"jobs": config.get("jobs", 1)} + if "child" in config: + options["child"] = config["child"] + server = server_module.build_server(app, **options) + async with stdio_server() as (read, write): + await server.run(read, write, server.create_initialization_options()) + + +if __name__ == "__main__": + anyio.run(main, json.loads(Path(sys.argv[1]).read_text())) diff --git a/tests/python/test_layering.py b/tests/python/test_layering.py index d7fa158f..30a435cf 100644 --- a/tests/python/test_layering.py +++ b/tests/python/test_layering.py @@ -95,10 +95,12 @@ " final_check gauge grid_domain hydrography io.cog io.domain_file io.geojson" " io.geopackage_write io.mesh_checks io.models io.ply io.repository io.rivers" " io.station_set" - " io.vtk_legacy land_cover_fractions landcover mosaic palettes raster run_record" - " sources stats" - " target_grid tin_engine tolerance_field viz.fixtures viz.protocols viz.scene" - " viz.style viz.svg", + " io.vtk_legacy land_cover_fractions landcover mcp.server mosaic palettes raster" + " run_record sources stats target_grid tin_engine tolerance_field viz.fixtures" + " viz.protocols viz.scene viz.style viz.svg", + "__main__": "cli", # 37: `python -m tin_engine`, the server's child + "mcp": "", "mcp.schema": "", # 37: the MCP server over the commands + "mcp.server": "mcp.schema tin_engine", }, ) # fmt: on diff --git a/tests/python/test_mcp_cli.py b/tests/python/test_mcp_cli.py new file mode 100644 index 00000000..dffe0ab9 --- /dev/null +++ b/tests/python/test_mcp_cli.py @@ -0,0 +1,44 @@ +"""Increment 37, T11: ``rasputin mcp`` is a command whether or not the extra is installed. + +`docs/increments/37-mcp-server.md`, sections 6.1, 6.8 and 9. The SDK is the +optional extra ``rasputin[mcp]``; without it, ``rasputin mcp`` refuses with +exit 2 and the install line, and every other command is unchanged. This file +does not skip when ``mcp`` is absent: that is the case it tests. The import is +made to fail by planting ``None`` for the SDK's packages in ``sys.modules``, +and the server module an earlier test may have imported is dropped, so the +command's lazy import cannot be satisfied from the cache. +""" + +from __future__ import annotations + +import re +import sys + +import pytest + +from cli_driver import USAGE, invoke, plain, runner +from tin_engine.cli import app + +#: The SDK's two top-level packages (`mcp` and its types), both absent without the extra. +SDK_PACKAGES = ("mcp", "mcp_types") +SERVER_MODULES = ("tin_engine.mcp.server", "tin_engine.mcp.schema", "tin_engine.mcp") + + +def test_mcp_refuses_with_the_install_line_when_the_extra_is_absent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + for name in SDK_PACKAGES: + monkeypatch.setitem(sys.modules, name, None) + for name in SERVER_MODULES: + monkeypatch.delitem(sys.modules, name, raising=False) + code, output = invoke("mcp") + assert code == USAGE, output + assert "Traceback" not in output + assert "the mcp extra is not installed" in output, output + assert 'pip install "rasputin[mcp]"' in output, output + + +def test_help_lists_mcp_beside_the_other_commands() -> None: + result = runner.invoke(app, ["--help"]) + assert result.exit_code == 0, result.output + assert re.search(r"\bmcp\b", plain(result.output)), plain(result.output) diff --git a/tests/python/test_mcp_schema.py b/tests/python/test_mcp_schema.py new file mode 100644 index 00000000..7be92b2b --- /dev/null +++ b/tests/python/test_mcp_schema.py @@ -0,0 +1,343 @@ +"""Increment 37, T1 to T3: each tool's schema is derived from its Typer command. + +`docs/increments/37-mcp-server.md`, sections 5, 6.3 and 9. `derive(app)` +reads the click parameters of every command and gives, per command, a tool +name, a Pydantic model (extra forbidden) and the way back to argv; the schema +is the model's JSON Schema. T1 and T2 use a toy app with no kernel behind it, +one parameter per row of section 5's type table; T3 derives the real app and +compares each schema with the command's own parameters at test time, so the +schema and the CLI cannot drift apart unnoticed. + +Per-property ``title`` (Pydantic's, from the field name) is ignored in every +comparison: it carries nothing the name does not. +""" + +from __future__ import annotations + +import enum +import json +from pathlib import Path +from typing import Annotated, Any + +import pytest + +pytest.importorskip("mcp") + +import typer +from pydantic import ValidationError + +from mcp_driver import tool_names +from tin_engine.cli import app +from tin_engine.mcp import schema as mcp_schema + +# ---------------------------------------------------------------- the toy app + + +class Mode(enum.StrEnum): + fast = "fast" + slow = "slow" + + +def toy_app() -> typer.Typer: + """Two commands covering section 5's type table, and the server's own command.""" + toy = typer.Typer() + + @toy.command() + def make_thing( + name: Annotated[str, typer.Argument(help="What to make.")], + count: Annotated[int, typer.Option("--count", help="How many.")] = 3, + level: Annotated[int, typer.Option("--level", min=1, max=5, help="One to five.")] = 2, + ratio: Annotated[float, typer.Option("--ratio", help="A share.")] = 0.5, + binary: Annotated[bool, typer.Option("--binary/--ascii", help="Binary or text.")] = True, + box: Annotated[ + tuple[float, float, float, float] | None, + typer.Option("--box", help="XMIN YMIN XMAX YMAX."), + ] = None, + near: Annotated[tuple[Path, float] | None, typer.Option("--near", help="FILE N.")] = None, + tag: Annotated[list[str] | None, typer.Option("--tag", help="Repeatable.")] = None, + mode: Annotated[Mode, typer.Option("--mode", help="A choice.")] = Mode.fast, + note: Annotated[str | None, typer.Option("--note", help="Optional text.")] = None, + where: Annotated[Path, typer.Option("--where", help="A path.")] = Path("here"), + ) -> None: + """Make a thing. + + The second paragraph is not the description. + """ + + @toy.command() + def plain() -> None: + """No parameters at all.""" + + @toy.command() + def mcp() -> None: + """The server's own command: not a tool.""" + + return toy + + +NUMBER = {"type": "number"} +STRING = {"type": "string"} +#: Section 5's type table, one property per row, as JSON Schema without ``title``. +EXPECTED_PROPERTIES: dict[str, dict[str, Any]] = { + "name": {**STRING, "description": "What to make."}, + "count": {"type": "integer", "default": 3, "description": "How many."}, + "level": { + "type": "integer", "minimum": 1, "maximum": 5, "default": 2, "description": "One to five.", + }, + "ratio": {**NUMBER, "default": 0.5, "description": "A share."}, + "binary": {"type": "boolean", "default": True, "description": "Binary or text."}, + "box": { + "anyOf": [ + {"type": "array", "prefixItems": [NUMBER] * 4, "minItems": 4, "maxItems": 4}, + {"type": "null"}, + ], + "default": None, + "description": "XMIN YMIN XMAX YMAX.", + }, + "near": { + "anyOf": [ + {"type": "array", "prefixItems": [STRING, NUMBER], "minItems": 2, "maxItems": 2}, + {"type": "null"}, + ], + "default": None, + "description": "FILE N.", + }, + "tag": { + "anyOf": [{"type": "array", "items": STRING}, {"type": "null"}], + "default": None, + "description": "Repeatable.", + }, + "mode": { + "enum": ["fast", "slow"], "type": "string", "default": "fast", "description": "A choice.", + }, + "note": {"anyOf": [STRING, {"type": "null"}], "default": None, "description": "Optional text."}, + "where": {**STRING, "default": "here", "description": "A path."}, +} # fmt: skip + + +def without_titles(properties: dict[str, dict[str, Any]]) -> dict[str, dict[str, Any]]: + return { + name: {k: v for k, v in prop.items() if k != "title"} for name, prop in properties.items() + } + + +def derived(application: typer.Typer) -> dict[str, Any]: + """`derive(app)` by tool name.""" + return {d.tool_name: d for d in mcp_schema.derive(application)} + + +def click_command(application: typer.Typer, tool_name: str) -> Any: + return typer.main.get_command(application).commands[tool_name.replace("_", "-")] + + +# ---------------------------------------------------------------- T1 + + +class TestSchemaFromAToyApp: + def test_one_tool_per_command_named_with_underscores_and_the_server_excluded(self) -> None: + tools = derived(toy_app()) + assert list(tools) == ["make_thing", "plain"] + assert tools["make_thing"].tool().name == "make_thing" + + def test_every_row_of_the_type_table(self) -> None: + schema = derived(toy_app())["make_thing"].tool().input_schema + assert schema["type"] == "object" + assert "title" not in schema + assert schema["additionalProperties"] is False + assert schema["required"] == ["name"] + assert without_titles(schema["properties"]) == EXPECTED_PROPERTIES + + def test_a_command_without_parameters(self) -> None: + schema = derived(toy_app())["plain"].tool().input_schema + assert schema["properties"] == {} + assert schema.get("required", []) == [] + assert schema["additionalProperties"] is False + + def test_the_description_is_the_docstrings_first_paragraph(self) -> None: + tools = derived(toy_app()) + assert tools["make_thing"].tool().description == "Make a thing." + assert tools["plain"].tool().description == "No parameters at all." + + @pytest.mark.parametrize( + "arguments", + [ + pytest.param({}, id="missing required argument"), + pytest.param({"name": "a", "box": [1.0, 2.0, 3.0]}, id="tuple of the wrong arity"), + pytest.param({"name": "a", "count": "many"}, id="wrong type"), + pytest.param({"name": "a", "bogus": 1}, id="unknown key"), + pytest.param({"name": "a", "mode": "medium"}, id="outside the choice"), + pytest.param({"name": "a", "level": 6}, id="above the range"), + ], + ) + def test_the_model_refuses_what_the_cli_would(self, arguments: dict[str, Any]) -> None: + model = derived(toy_app())["make_thing"].model + with pytest.raises(ValidationError): + model.model_validate(arguments) + + +# ---------------------------------------------------------------- T2 + + +class TestArgvRoundTrip: + @pytest.mark.parametrize( + ("arguments", "argv"), + [ + pytest.param({"name": "a"}, ["make-thing", "a"], id="only set keys appear"), + pytest.param( + {"name": "a", "binary": False}, ["make-thing", "a", "--ascii"], id="false flag" + ), + pytest.param( + {"name": "a", "binary": True}, ["make-thing", "a", "--binary"], id="true flag" + ), + pytest.param( + {"name": "a", "box": [1.5, 2.5, 3.5, 4.5]}, + ["make-thing", "a", "--box", "1.5", "2.5", "3.5", "4.5"], + id="a tuple expands", + ), + pytest.param( + {"name": "a", "near": ["lines.geojson", 2.5]}, + ["make-thing", "a", "--near", "lines.geojson", "2.5"], + id="a path-and-number tuple", + ), + pytest.param( + {"name": "a", "tag": ["x", "y"]}, + ["make-thing", "a", "--tag", "x", "--tag", "y"], + id="a list repeats the option", + ), + pytest.param( + {"mode": "slow", "count": 7, "name": "a"}, + ["make-thing", "a", "--count", "7", "--mode", "slow"], + id="the command's parameter order, not the caller's", + ), + pytest.param( + {"name": "a", "where": "there/x.vtk"}, + ["make-thing", "a", "--where", "there/x.vtk"], + id="a path is passed as given", + ), + ], + ) + def test_argv_is_the_command_line_the_arguments_mean( + self, arguments: dict[str, Any], argv: list[str] + ) -> None: + tool = derived(toy_app())["make_thing"] + assert tool.argv(tool.model.model_validate(arguments)) == argv + + def test_a_command_without_parameters_is_its_name(self) -> None: + tool = derived(toy_app())["plain"] + assert tool.argv(tool.model.model_validate({})) == ["plain"] + + +# ---------------------------------------------------------------- T3 + +#: Section 1's eight commands as tool names. Their parameter counts are not +#: pinned: the test below holds each schema's properties equal to the command's +#: own click parameters, which implies the count, and a pinned count would turn +#: red on every master merge that adds a CLI option (as `mesh` 34 -> 36 did when +#: `--land-cover` and `--land-cover-year` landed) without saying anything about +#: the schema. +TOOL_NAMES = { + "mesh", "draw", "fetch", "catchment", "station_catchments", + "fetch_stations", "palette", "version", +} # fmt: skip + +#: Section 3's snapshots of five `mesh` properties, without ``title``. +MESH_SNAPSHOTS: dict[str, dict[str, Any]] = { + "bbox": { + "anyOf": [ + {"maxItems": 4, "minItems": 4, "prefixItems": [NUMBER] * 4, "type": "array"}, + {"type": "null"}, + ], + "default": None, + "description": "With --dem, mesh only XMIN YMIN XMAX YMAX in the DEM's CRS, snapped " + "outward to its nodes.", + }, + "tolerance_near": { + "anyOf": [ + {"maxItems": 2, "minItems": 2, "prefixItems": [STRING, NUMBER], "type": "array"}, + {"type": "null"}, + ], + "default": None, + "description": "Hold the mesh to N metres on these lines (.geojson, .gpkg or .gml). " + "Needs --tolerance-ramp.", + }, + "binary": { + "default": True, + "description": "Binary files, small and fast; --ascii writes text you can read.", + "type": "boolean", + }, + "features": { + "anyOf": [{"items": STRING, "type": "array"}, {"type": "null"}], + "default": None, + "description": "With --domain, polygons and lines as constraints (.geojson, .json, " + ".gpkg or .gml), clipped to the domain, each edge carrying its class map's bits. " + "Repeatable: each --features is one source, paired by position with the Nth of " + "--features-crs, --features-layer and --features-map.", + }, + "snap_spacing": { + "default": 0.001, + "description": "Snap grid spacing for the noder, in the input's units.", + "type": "number", + }, +} + +RUN_RESULT_FIELDS = { + "command", + "exit_code", + "seconds", + "outputs", + "stdout", + "stderr", + "record", + "stats", +} + + +class TestTheRealApp: + def test_the_eight_commands_in_the_clis_order(self) -> None: + tools = derived(app) + assert list(tools) == tool_names(app) + assert set(tools) == TOOL_NAMES + + def test_no_drift_from_the_click_parameters(self) -> None: + tools = derived(app) + for name, d in tools.items(): + params = {p.name for p in click_command(app, name).params} + assert set(d.tool().input_schema["properties"]) == params, name + + def test_mesh_requires_out_and_matches_the_snapshots(self) -> None: + schema = derived(app)["mesh"].tool().input_schema + assert schema["required"] == ["out"] + assert schema["additionalProperties"] is False + properties = without_titles(schema["properties"]) + assert {k: properties[k] for k in MESH_SNAPSHOTS} == MESH_SNAPSHOTS + + def test_an_integer_range_carries_its_minimum(self) -> None: + connections = derived(app)["fetch"].tool().input_schema["properties"]["connections"] + assert {k: connections[k] for k in ("type", "minimum", "default")} == { + "type": "integer", "minimum": 1, "default": 8, + } # fmt: skip + + def test_the_whole_tool_list_stays_small(self) -> None: + # 16,575 bytes measured (section 3); the bound leaves room for help text to grow. + size = sum(len(json.dumps(d.tool().input_schema)) for d in mcp_schema.derive(app)) + assert size < 20_000, size + + def test_descriptions_and_annotations(self) -> None: + tools = {name: d.tool() for name, d in derived(app).items()} + assert tools["version"].description == ( + "Print the installed rasputin version, then whether it is bounds-checked." + ) + version = tools["version"].annotations + assert version is not None + assert (version.read_only_hint, version.idempotent_hint) == (True, True) + for name in ("fetch", "fetch_stations"): + annotations = tools[name].annotations + assert annotations is not None and annotations.open_world_hint is True, name + for name, tool in tools.items(): + assert tool.annotations is not None and tool.annotations.destructive_hint is False, name + + def test_every_tool_declares_the_run_result_as_its_output(self) -> None: + for d in mcp_schema.derive(app): + output = d.tool().output_schema + assert output is not None, d.tool_name + assert set(output["properties"]) >= RUN_RESULT_FIELDS, d.tool_name diff --git a/tests/python/test_mcp_server.py b/tests/python/test_mcp_server.py new file mode 100644 index 00000000..ef87620b --- /dev/null +++ b/tests/python/test_mcp_server.py @@ -0,0 +1,405 @@ +"""Increment 37, T4 to T6, T8 and T10 in process: the server runs the CLI as a child. + +`docs/increments/37-mcp-server.md`, sections 5, 6.4 to 6.6, 7 and 9. The +SDK's `Client(server)` connects in process; the child is `mcp_fake_child` +where the behaviour under test is the server's (validation, refusal mapping, +sidecar removal, cancellation, the jobs cap) and the real ``python -m +tin_engine`` where it is the whole round trip (T6, the micro tile and the +Kartverket tile). + +Clocks: every bound here is on the 0.5 s fake child and the 0.72 s probe run +(section 3), with the slack each comment states; nothing waits on the wall +clock without a bound. +""" + +from __future__ import annotations + +import asyncio +import json +import time +from collections.abc import Callable +from pathlib import Path +from typing import Any + +import pytest + +pytest.importorskip("mcp") + +from mcp.client import Client +from mcp.shared.exceptions import MCPError + +from geotiff_fixtures import KARTVERKET, micro_tiff, needs_codecs +from mcp_driver import ( + RECORD_SIDECAR, + STATS_SIDECAR, + Progress, + call, + fake_child, + intervals, + only_files, + option_value, + overlap, + resource_links, + structured, + text_block, + wait_until_dead, +) +from tin_engine.cli import app +from tin_engine.mcp import server as mcp_server +from tin_engine.run_record import WORDING +from vtkread import read_vtk + +Build = Callable[..., Any] + + +@pytest.fixture +def build() -> Build: + """`build_server` on the real app; ``child`` and ``jobs`` as the test says.""" + + def _build(**options: Any) -> Any: + return mcp_server.build_server(app, **options) + + return _build + + +@pytest.fixture +def out_dir(tmp_path: Path) -> Path: + """Where ``out`` goes: its own directory, so a leftover sidecar is visible.""" + directory = tmp_path / "out" + directory.mkdir() + return directory + + +@pytest.fixture +def micro_dem(tmp_path: Path) -> Path: + """The 3 x 4 node tile at 10 m x 5 m (`geotiff_fixtures`), no codec needed.""" + path = tmp_path / "micro.tif" + path.write_bytes(micro_tiff().getvalue()) + return path + + +def mesh_arguments(dem: Path, out: Path | str, **more: Any) -> dict[str, Any]: + return {"dem": [str(dem)], "out": str(out), **more} + + +# ---------------------------------------------------------------- T4 + + +class TestValidationBeforeTheChild: + async def test_a_missing_out_is_the_models_refusal_verbatim( + self, tmp_path: Path, build: Build + ) -> None: + argv_file = tmp_path / "argv.json" + server = build(child=fake_child(tmp_path, argv_file=str(argv_file))) + result = await call(server, "mesh", {}) + assert result.is_error + assert structured(result) == {"exit_code": None, "error": "out: Field required"} + assert "out: Field required" in text_block(result) + assert not argv_file.exists(), "the child ran on arguments the model should have refused" + + @pytest.mark.parametrize( + ("arguments", "error_prefix"), + [ + pytest.param( + {"out": "a.vtk", "tolerance": "ten"}, + "tolerance: Input should be a valid number", + id="wrong type", + ), + pytest.param( + {"out": "a.vtk", "bogus": 1}, + "bogus: Extra inputs are not permitted", + id="unknown key", + ), + pytest.param({"out": "a.vtk", "bbox": [1.0, 2.0, 3.0]}, "bbox: ", id="bbox of three"), + ], + ) + async def test_refused_naming_the_field_and_the_child_never_runs( + self, tmp_path: Path, build: Build, arguments: dict[str, Any], error_prefix: str + ) -> None: + argv_file = tmp_path / "argv.json" + server = build(child=fake_child(tmp_path, argv_file=str(argv_file))) + result = await call(server, "mesh", arguments) + assert result.is_error + body = structured(result) + assert body["exit_code"] is None + assert body["error"].startswith(error_prefix), body + assert not argv_file.exists() + + async def test_out_parent_passes_through_and_the_record_sidecar_is_beside_out( + self, tmp_path: Path, out_dir: Path, build: Build + ) -> None: + argv_file = tmp_path / "argv.json" + server = build(child=fake_child(tmp_path, argv_file=str(argv_file))) + await call(server, "mesh", {"out": str(out_dir / "a.vtk"), "out_parent": str(out_dir)}) + argv = json.loads(argv_file.read_text()) + assert argv[0] == "mesh" + assert option_value(argv, "--out") == str(out_dir / "a.vtk") + assert option_value(argv, "--out-parent") == str(out_dir) + record = Path(option_value(argv, "--record")) + assert record.parent == out_dir + match = RECORD_SIDECAR.fullmatch(record.name) + assert match is not None and match["out"] == "a.vtk", record.name + assert "--stats" not in argv + + async def test_stats_dash_becomes_a_stats_sidecar_beside_out( + self, tmp_path: Path, out_dir: Path, build: Build + ) -> None: + argv_file = tmp_path / "argv.json" + server = build(child=fake_child(tmp_path, argv_file=str(argv_file))) + await call(server, "mesh", {"out": str(out_dir / "a.vtk"), "stats": "-"}) + argv = json.loads(argv_file.read_text()) + stats = Path(option_value(argv, "--stats")) + assert stats.parent == out_dir + match = STATS_SIDECAR.fullmatch(stats.name) + assert match is not None and match["out"] == "a.vtk", stats.name + assert RECORD_SIDECAR.fullmatch(Path(option_value(argv, "--record")).name) + + async def test_a_given_record_path_is_the_callers_and_no_sidecar_is_added( + self, tmp_path: Path, out_dir: Path, build: Build + ) -> None: + argv_file = tmp_path / "argv.json" + server = build(child=fake_child(tmp_path, argv_file=str(argv_file))) + keep = out_dir / "keep.json" + await call(server, "mesh", {"out": str(out_dir / "a.vtk"), "record_path": str(keep)}) + argv = json.loads(argv_file.read_text()) + assert argv.count("--record") == 1 + assert option_value(argv, "--record") == str(keep) + + +# ---------------------------------------------------------------- T5 + +REFUSAL = [ + "Usage: rasputin mesh [OPTIONS] [NAME]", + "Try 'rasputin mesh --help' for help.", + "", + "Error: Invalid value for --dem: cannot read /nonexistent.tif: No such file or directory", +] + + +class TestRefusalMapping: + async def test_exit_2_gives_the_error_lines_words_and_the_whole_stderr( + self, tmp_path: Path, out_dir: Path, build: Build + ) -> None: + server = build(child=fake_child(tmp_path, stderr=[[0, line] for line in REFUSAL], exit=2)) + result = await call( + server, "mesh", {"out": str(out_dir / "a.vtk"), "dem": ["/nonexistent.tif"]} + ) + assert result.is_error + body = structured(result) + assert body["exit_code"] == 2 + assert body["error"] == f"exit 2: {REFUSAL[-1].removeprefix('Error: ')}" + assert body["stderr"] == REFUSAL + assert "Invalid value for --dem" in text_block(result) + + async def test_exit_1_likewise(self, tmp_path: Path, build: Build) -> None: + words = "Error: the catalogue is unreachable" + server = build(child=fake_child(tmp_path, stderr=[[0, words]], exit=1)) + result = await call(server, "version", {}) + assert result.is_error + body = structured(result) + assert (body["exit_code"], body["error"]) == (1, "exit 1: the catalogue is unreachable") + + async def test_progress_lines_before_the_error_line_stay_out_of_error( + self, tmp_path: Path, build: Build + ) -> None: + """A `fetch` failing mid-run prints its progress first (`cli.py`, the ``bytes`` lines, + then the ``Error:`` line); `error` carries the ``Error:`` line's words, `stderr` all.""" + lines = ["1,000 of 2,000 bytes", "2,000 of 2,000 bytes", "Error: no route to the catalogue"] + server = build(child=fake_child(tmp_path, stderr=[[0, line] for line in lines], exit=1)) + result = await call(server, "fetch", {"source": "glo30"}) + assert result.is_error + body = structured(result) + assert (body["exit_code"], body["error"]) == (1, "exit 1: no route to the catalogue") + assert body["stderr"] == lines + + async def test_without_an_error_line_every_line_but_the_usage_reminder_is_kept( + self, tmp_path: Path, build: Build + ) -> None: + """The fallback: a child that dies without an ``Error:`` line (a traceback, an + ``Aborted!``) still has its words in `error`, less ``Usage:`` and ``Try ...``.""" + lines = [ + "Usage: rasputin fetch [OPTIONS] {source}", + "Try 'rasputin fetch --help' for help.", + "", + "1,000 of 2,000 bytes", + "Aborted!", + ] + server = build(child=fake_child(tmp_path, stderr=[[0, line] for line in lines], exit=1)) + result = await call(server, "fetch", {"source": "glo30"}) + assert result.is_error + body = structured(result) + assert (body["exit_code"], body["error"]) == (1, "exit 1: 1,000 of 2,000 bytes Aborted!") + assert body["stderr"] == lines + + async def test_a_signal_is_named(self, tmp_path: Path, build: Build) -> None: + server = build(child=fake_child(tmp_path, signal=15)) + result = await call(server, "version", {}) + assert result.is_error + body = structured(result) + assert (body["exit_code"], body["error"]) == (-15, "killed by signal 15 (SIGTERM)") + + @pytest.mark.parametrize("create_record", [False, True], ids=["no sidecar", "sidecar created"]) + async def test_no_record_sidecar_remains_after_a_refusal( + self, tmp_path: Path, out_dir: Path, build: Build, create_record: bool + ) -> None: + server = build( + child=fake_child( + tmp_path, stderr=[[0, REFUSAL[-1]]], exit=2, create_record=create_record + ) + ) + result = await call(server, "mesh", {"out": str(out_dir / "a.vtk")}) + assert result.is_error + only_files(out_dir, []) + + +# ---------------------------------------------------------------- T6 + + +def assert_micro_mesh(result: Any, out: Path) -> dict[str, Any]: + """A successful `mesh` of the micro tile to ``out``: the result, as section 5 shapes it.""" + assert not result.is_error, result.content + body = structured(result) + assert body["exit_code"] == 0 + assert body["command"].startswith("rasputin mesh "), body["command"] + assert body["seconds"] > 0 + assert [Path(p).resolve() for p in body["outputs"]] == [out.resolve()] + record = body["record"] + assert isinstance(record, dict) + assert list(record)[:2] == ["rasputin_version", "command"] + assert list(record)[2:] == [k for k in WORDING if k in record], "the record's own order" + assert body["stderr"][0].startswith("12 triangles"), body["stderr"] + vtk = read_vtk(out.read_bytes()) + assert len(vtk.polygons) == 12 # 3 x 4 nodes: 2 x 3 cells, two triangles each + assert json.loads(text_block(result)) == body + assert resource_links(result) == [(out.resolve().as_uri(), "application/octet-stream")] + return body + + +class TestRealMesh: + @pytest.mark.parametrize("with_out_parent", [False, True], ids=["plain", "out_parent"]) + async def test_mesh_writes_the_vtk_returns_the_record_and_leaves_no_sidecar( + self, out_dir: Path, micro_dem: Path, build: Build, with_out_parent: bool + ) -> None: + out = out_dir / "a.vtk" + extra = {"out_parent": str(out_dir)} if with_out_parent else {} + result = await call(build(), "mesh", mesh_arguments(micro_dem, out, **extra)) + body = assert_micro_mesh(result, out) + assert body["stats"] is None + only_files(out_dir, ["a.vtk"]) + + async def test_out_outside_out_parent_is_refused_naming_out_not_the_sidecar( + self, tmp_path: Path, out_dir: Path, micro_dem: Path, build: Build + ) -> None: + sibling = tmp_path / "sibling" + sibling.mkdir() + out = out_dir / "a.vtk" + result = await call( + build(), "mesh", mesh_arguments(micro_dem, out, out_parent=str(sibling)) + ) + assert result.is_error + body = structured(result) + assert body["exit_code"] == 2 + assert "--out" in body["error"], body["error"] + assert "--record" not in body["error"] and "--stats" not in body["error"], body["error"] + only_files(out_dir, []) + only_files(sibling, []) + + async def test_a_relative_out_lands_in_the_servers_working_directory( + self, out_dir: Path, micro_dem: Path, build: Build, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.chdir(out_dir) + result = await call(build(), "mesh", mesh_arguments(micro_dem, "rel.vtk")) + assert_micro_mesh(result, out_dir / "rel.vtk") + only_files(out_dir, ["rel.vtk"]) + + async def test_stats_dash_returns_the_report_as_data_not_on_stdout( + self, out_dir: Path, micro_dem: Path, build: Build + ) -> None: + out = out_dir / "a.vtk" + result = await call(build(), "mesh", mesh_arguments(micro_dem, out, stats="-")) + body = assert_micro_mesh(result, out) + assert body["stats"].startswith("# rasputin mesh — statistics"), body["stats"][:80] + assert "## Sizes" in body["stats"] + markdown = [line for line in body["stdout"] if line.startswith(("#", "|", "`"))] + assert markdown == [], body["stdout"] + only_files(out_dir, ["a.vtk"]) + + @needs_codecs + async def test_the_kartverket_tile_at_one_metre( + self, out_dir: Path, build: Build, capsys: pytest.CaptureFixture[str] + ) -> None: + out = out_dir / "k.vtk" + began = time.perf_counter() + result = await call(build(), "mesh", mesh_arguments(KARTVERKET, out, tolerance=1.0)) + seconds = time.perf_counter() - began + assert not result.is_error, result.content + body = structured(result) + record = body["record"] + assert record["tolerance_m"] == 1 + assert record["max_error_m"] <= 1 + triangles = int(body["stderr"][0].split()[0]) + # 464,290 measured on the probe's machine (section 3); the existing real-tile + # test pins no exact count, so a 12 % window either side does for this one. + assert 410_000 <= triangles <= 520_000, body["stderr"] + only_files(out_dir, ["k.vtk"]) + with capsys.disabled(): + print(f"\nT6 Kartverket through the tool: {triangles} triangles, {seconds:.2f} s") + + +# ---------------------------------------------------------------- T8, T9 in process + + +class TestCancellation: + async def test_a_client_timeout_kills_the_child(self, tmp_path: Path, build: Build) -> None: + pid_file = tmp_path / "child.pid" + server = build(child=fake_child(tmp_path, pid_file=str(pid_file), sleep=30)) + async with Client(server) as client: + with pytest.raises(MCPError): + await client.call_tool("version", {}, read_timeout_seconds=0.5) + # measured: dead 0.5 s after a 1.0 s timeout (section 3, probe C) + await wait_until_dead(pid_file, within=1.0) + + +async def test_in_process_a_call_with_a_progress_callback_completes( + tmp_path: Path, build: Build +) -> None: + # The in-process transport carries no progress token (section 2, measured), so + # the notifications are asserted over stdio (test_mcp_stdio); here only that the + # run completes with a callback attached. + server = build(child=fake_child(tmp_path, stderr=[[0, "a line"]])) + result = await call(server, "version", {}, progress_callback=Progress()) + assert structured(result)["exit_code"] == 0 + + +# ---------------------------------------------------------------- T10 + + +class TestJobsCap: + async def test_jobs_1_runs_two_calls_one_after_the_other( + self, tmp_path: Path, build: Build + ) -> None: + log = tmp_path / "intervals.jsonl" + server = build(jobs=1, child=fake_child(tmp_path, sleep=0.5, interval_file=str(log))) + async with Client(server) as client: + began = time.monotonic() + first, second = await asyncio.gather( + client.call_tool("version", {}), client.call_tool("version", {}) + ) + wall = time.monotonic() - began + assert not first.is_error and not second.is_error + runs = intervals(log) + assert len(runs) == 2 and not overlap(*runs), runs + assert wall >= 1.0, wall # two 0.5 s sleeps in sequence; start-up only adds + + async def test_jobs_2_runs_them_side_by_side(self, tmp_path: Path, build: Build) -> None: + log = tmp_path / "intervals.jsonl" + server = build(jobs=2, child=fake_child(tmp_path, sleep=0.5, interval_file=str(log))) + async with Client(server) as client: + began = time.monotonic() + await asyncio.gather(client.call_tool("version", {}), client.call_tool("version", {})) + wall = time.monotonic() - began + runs = intervals(log) + assert len(runs) == 2 and overlap(*runs), runs + # two 0.5 s sleeps side by side: about 0.55 s with the interpreter's start-up; + # the bound leaves 0.45 s for a loaded machine and still refutes a sequence (1.0 s) + assert wall < 1.0, wall diff --git a/tests/python/test_mcp_stdio.py b/tests/python/test_mcp_stdio.py new file mode 100644 index 00000000..df2a540e --- /dev/null +++ b/tests/python/test_mcp_stdio.py @@ -0,0 +1,142 @@ +"""Increment 37, T7 to T9 over stdio: the transport an agent's configuration uses. + +`docs/increments/37-mcp-server.md`, sections 6.5, 6.6, 6.8 and 9. The SDK's +client spawns the server as a process and speaks JSON-RPC on its pipes, so +this is where a stray write to the server's stdout would show, and the only +transport that carries a progress token (section 2, measured). T7 spawns the +production entry, ``python -m tin_engine mcp``; T8 and T9 spawn +`mcp_stdio_launcher` with `mcp_fake_child` as the child. + +Clocks: a progress value is seconds since the handler began. It is bounded on +both sides without a tolerance: at least the sleeps the child has done before +the line (0.2 s each), and at most the client's own elapsed time at receipt, +since the handler began after the call was sent. A line count would fail the +upper bound at the first line. +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +pytest.importorskip("mcp") + +import anyio +from mcp.client import Client +from mcp.shared.exceptions import MCPError + +from mcp_driver import ( + RUNNING, + WAITING, + Progress, + fake_child, + production_server, + stdio_server, + structured, + tool_names, + wait_until_dead, +) +from tin_engine.cli import app + +# ---------------------------------------------------------------- T7 + + +async def test_python_m_tin_engine_mcp_lists_the_tools_and_runs_version() -> None: + # connect and initialize measured at 0.465 s, a version call at 0.264 s (section 3) + with anyio.fail_after(30): + async with Client(production_server()) as client: + listed = await client.list_tools() + result = await client.call_tool("version", {}) + assert [tool.name for tool in listed.tools] == tool_names(app) + assert not result.is_error, result.content + body = structured(result) + assert body["exit_code"] == 0 + assert len(body["stdout"]) == 2, body["stdout"] + assert body["stdout"][1].startswith("bounds checks: ") + assert body["outputs"] == [] + + +# ---------------------------------------------------------------- T8 + + +async def test_a_client_timeout_kills_the_child_over_stdio(tmp_path: Path) -> None: + pid_file = tmp_path / "child.pid" + params = stdio_server(tmp_path, child=fake_child(tmp_path, pid_file=str(pid_file), sleep=30)) + async with Client(params) as client: + with pytest.raises(MCPError): + await client.call_tool("version", {}, read_timeout_seconds=0.5) + # measured: dead 0.5 s after a 1.0 s timeout (section 3, probe C) + await wait_until_dead(pid_file, within=1.0) + + +# ---------------------------------------------------------------- T9 + + +class TestProgress: + async def test_each_stderr_line_is_a_notification_carrying_the_line( + self, tmp_path: Path + ) -> None: + lines = ["one", "two", "three"] + params = stdio_server( + tmp_path, child=fake_child(tmp_path, stderr=[[0.2, s] for s in lines]) + ) + async with Client(params) as client: + progress = Progress() + result = await client.call_tool("version", {}, progress_callback=progress) + assert structured(result)["stderr"] == lines + assert progress.messages == lines + progress.assert_strictly_increasing() + progress.assert_elapsed_seconds() + for i, value in enumerate(progress.values, start=1): + assert value >= 0.2 * i, (i, value) # i sleeps of 0.2 s came before line i + + async def test_heartbeats_say_running_with_elapsed_seconds(self, tmp_path: Path) -> None: + params = stdio_server( + tmp_path, + heartbeat=0.1, + child=fake_child(tmp_path, stderr=[[0.25, "phase"]], sleep=0.35), + ) + async with Client(params) as client: + progress = Progress() + result = await client.call_tool("version", {}, progress_callback=progress) + assert structured(result)["exit_code"] == 0 + messages = [m or "" for m in progress.messages] + assert all(m == "phase" or RUNNING.fullmatch(m) for m in messages), messages + assert messages.count("phase") == 1 + line = messages.index("phase") + # heartbeats every 0.1 s over a 0.6 s run: five expected, at least three asserted, + # one before the line at 0.25 s and one after (a loaded machine delays, never adds) + assert len(messages) - 1 >= 3, messages + assert 0 < line < len(messages) - 1, messages + progress.assert_strictly_increasing() + progress.assert_elapsed_seconds() + + async def test_a_queued_call_reports_waiting_then_running_on_one_increasing_clock( + self, tmp_path: Path + ) -> None: + params = stdio_server( + tmp_path, jobs=1, heartbeat=0.1, child=fake_child(tmp_path, sleep=0.5) + ) + first, second = Progress(), Progress() + async with Client(params) as client: + results = await asyncio.gather( + client.call_tool("version", {}, progress_callback=first), + client.call_tool("version", {}, progress_callback=second), + ) + assert all(structured(r)["exit_code"] == 0 for r in results) + waited = [p for p in (first, second) if any(WAITING.fullmatch(m or "") for m in p.messages)] + assert len(waited) == 1, "of two concurrent calls, exactly one queues behind the other" + (queued,) = waited + kinds = ["waiting" if WAITING.fullmatch(m or "") else "running" for m in queued.messages] + assert all(RUNNING.fullmatch(m or "") for m, k in zip(queued.messages, kinds, strict=True) + if k == "running"), queued.messages # fmt: skip + # the first call holds the semaphore for 0.5 s plus start-up: at least three + # waiting heartbeats at 0.1 s, then the running ones, never interleaved + assert kinds.count("waiting") >= 3 and "running" in kinds, kinds + assert kinds == sorted(kinds, key=("waiting", "running").index), kinds + queued.assert_strictly_increasing() + queued.assert_elapsed_seconds() + for progress in (first, second): + progress.assert_strictly_increasing()