Skip to content

Commit 557525a

Browse files
aborrusoclaude
andauthored
test(smoke): a release gate that asserts which dataset comes back (#538)
* test(smoke): a release gate that asserts which dataset comes back Three releases went out today, two to repair the one before. The verification between them was a sequence of commands rebuilt from memory, so each pass covered something different — and every pass covered result counts. v0.4.122 shipped with counts verified and ranking broken: "22 results" and "the right dataset first" are different claims, and only the second is what a caller asked for. `npm run smoke` makes that check a command. Twelve known-answer cases drawn from real telemetry, each asserting which dataset must come back and naming the regression it guards; the runner starts the built server over HTTP and calls each tool the way a client does. Checked against the defects it claims to catch: reintroducing the v0.4.121 wrapping rule fails 4 of 12 cases, and removing the parser probe from ckan_find_relevant_datasets — the v0.4.122 regression — fails the case requiring the two search tools to agree. Wired into CLAUDE.md step 2, before the tag, and into the DEPLOYMENT.md checklist. Two things the gate needed: - the JSON format never exposed the query that actually ran, while Markdown always has. `effective_query` now appears in ckan_package_search JSON when the server rewrote the query, absent when it did not. Documented in docs/JSON-OUTPUT.md. - relevance scores summed unrounded fractions, printing 8.299999999999999. 536 tests, 1 added. Not fixed, and recorded in LOG.md: the ranking model weights every query term equally, so three datasets tie at 9.7 on the Lecce query and the right one leads on Solr order. Making `tags` proportional was tried and reverted — it promotes a dataset whose tags carry two of the three terms over the one that actually answers the question. Term specificity is a design change, not a patch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 * test(smoke): build before running, fail loudly on setup, compare tools for real Four review findings: - `npm run smoke` launched dist/index.js without building it. On a fresh checkout there is nothing to launch; on a stale one it tests yesterday's code and calls it a release gate. It builds first now. - the runner suppressed the child's stderr, ignored its exit, and waited a fixed 1.5s. A missing build or an occupied port surfaced as twelve cases failing with `fetch failed`, which reads like a broken server rather than a runner that never started one. It now waits for `tools/list` to answer — the Node HTTP transport exposes only /mcp, so a health path would never have worked — reports the child's exit and last stderr lines, and exits 2 for a setup failure against 1 for a real one. - the case named "both search tools agree" called one tool and checked its count against a threshold. It now calls both and compares: with the v0.4.122 regression reintroduced it reports "ckan_package_search reports 87 and ckan_find_relevant_datasets reports 0". - `effective_query` is an externally visible response field and had no OpenSpec proposal. Added as openspec/changes/add-effective-query-json, `openspec validate --strict` green, with the scenarios for both branches — present when the query was rewritten, absent when it was not. 536 tests, 12/12 smoke. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 * test(smoke): readiness must recognise our server, not just a live socket The probe accepted any 200 from POST /mcp, so a foreign service holding SMOKE_PORT would have been treated as ready and the whole suite would have run against it — turning a bind failure into twelve puzzling case failures, which is exactly what the readiness check was added to prevent. It now requires `tools/list` to name `ckan_package_search`. Verified by putting a stub on 3099 that answers tools/list with a different tool: exit 2 and "something else is answering on port 3099: it replied to tools/list without offering ckan_package_search". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MEpSWpAwuMaGkfnMpQdqK2 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 19fe07f commit 557525a

13 files changed

Lines changed: 522 additions & 26 deletions

File tree

CLAUDE.md

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -210,16 +210,16 @@ The server (`src/index.ts`):
210210
- `tools/datastore.ts`: `ckan_datastore_search`
211211
- `tools/status.ts`: `ckan_status_show`
212212

213-
3. **MCP Resource Templates** (`resources/`)
213+
4. **MCP Resource Templates** (`resources/`)
214214
- `ckan://{server}/dataset/{id}` - Dataset metadata
215215
- `ckan://{server}/resource/{id}` - Resource metadata
216216
- `ckan://{server}/organization/{name}` - Organization metadata
217217

218-
4. **Utility Functions** (`utils/`)
218+
5. **Utility Functions** (`utils/`)
219219
- `http.ts`: `makeCkanRequest<T>()` - HTTP client for CKAN API v3
220220
- `formatting.ts`: `truncateText()`, `formatDate()`, `formatBytes()`
221221

222-
5. **Type Definitions** (`types.ts`)
222+
6. **Type Definitions** (`types.ts`)
223223
- `ResponseFormat` enum (MARKDOWN, JSON)
224224
- `ResponseFormatSchema` Zod validator
225225
- `CHARACTER_LIMIT` constant
@@ -228,11 +228,11 @@ The server (`src/index.ts`):
228228
- `stdio.ts`: Standard input/output (Claude Desktop)
229229
- `http.ts`: HTTP server (remote access)
230230

231-
6. **Validation Schema**
231+
7. **Validation Schema**
232232
- Uses Zod to validate all tool inputs
233233
- Each tool has a strict schema that rejects extra parameters
234234

235-
7. **Output Formatting**
235+
8. **Output Formatting**
236236
- All tools support two formats: `markdown` (default) and `json`
237237
- Markdown format optimized for human readability
238238
- JSON format returns compact objects with only essential fields (~70% token reduction vs raw CKAN API)
@@ -460,20 +460,21 @@ npm cannot resolve relative paths from the tarball.
460460
When releasing a new version:
461461

462462
1. **Update version**: Edit the version field in `package.json`, `package-lock.json`, `manifest.json`, **`server.json`** (in `server.json` there are **two** fields: top-level `version` and `packages[0].version` — both must match), **`src/server.ts`** (MCP server version) and **`src/worker.ts`** (`/health` response). The two source files are easy to forget: v0.4.119 shipped with them still at 0.4.118. Check with `grep -rn "<old version>" package.json manifest.json server.json src/server.ts src/worker.ts` — it must return nothing
463-
2. **Update LOG.md**: Add entry with date and changes
464-
3. **Commit changes on a branch**: `git checkout -b <type>/<description>` then `git add . && git commit -m "..."`. Code never goes straight to `main`; documentation-only changes may.
465-
4. **Open a PR and merge it**: `git push -u origin <branch>`, `gh pr create`, wait for green checks, `gh pr merge --squash --delete-branch`, then `git checkout main && git pull`. `main` has a `non_fast_forward` rule: a commit pushed there by mistake needs a revert, not a force-push.
466-
5. **Create tag**: `git tag -a v0.x.0 -m "..." && git push origin v0.x.0` — ⚠️ **this triggers the npm publish**, see step 9
467-
6. **Build DXT**: `npm run pack:dxt` → produces `ckan-mcp-server.dxt`
468-
7. **Build skill**: `npm run pack:skill` → produces `tmp/ckan-mcp.skill`
469-
8. **Attach to release**: `gh release upload v0.x.0 ckan-mcp-server.dxt tmp/ckan-mcp.skill`
470-
9. **npm publish happens automatically**: pushing the tag in step 5 starts `.github/workflows/release.yml`, which verifies the tag matches `package.json`, builds, tests, and runs `npm publish --provenance`. **Do not run `npm publish` by hand** — the two paths collide and the loser gets `EPUBLISHCONFLICT`. Watch the run: `gh run watch $(gh run list --workflow=release.yml --limit 1 --json databaseId -q '.[0].databaseId')`
471-
10. **Publish to the MCP Registry** — only **after** the step 9 run has finished green, since the registry validates that the npm version exists: `mcp-publisher login github && mcp-publisher publish`. If login fails with `incorrect_device_code`, the local binary is stale: compare `mcp-publisher --version` against the [latest release](https://github.com/modelcontextprotocol/registry/releases) and update it — the device-auth flow has changed between versions, and the error does not say so
472-
11. **Deploy to Cloudflare** (if code changed): `npm run deploy`
463+
2. **Run the release gate**: `npm run smoke` — known-answer search queries against live portals, asserting *which* dataset comes back rather than how many. It must be green before tagging. It exists because v0.4.122 shipped with counts verified and ranking broken: "22 results" and "the right dataset first" are different claims
464+
3. **Update LOG.md**: Add entry with date and changes
465+
4. **Commit changes on a branch**: `git checkout -b <type>/<description>` then `git add . && git commit -m "..."`. Code never goes straight to `main`; documentation-only changes may.
466+
5. **Open a PR and merge it**: `git push -u origin <branch>`, `gh pr create`, wait for green checks, `gh pr merge --squash --delete-branch`, then `git checkout main && git pull`. `main` has a `non_fast_forward` rule: a commit pushed there by mistake needs a revert, not a force-push.
467+
6. **Create tag**: `git tag -a v0.x.0 -m "..." && git push origin v0.x.0` — ⚠️ **this triggers the npm publish**, see step 10
468+
7. **Build DXT**: `npm run pack:dxt` → produces `ckan-mcp-server.dxt`
469+
8. **Build skill**: `npm run pack:skill` → produces `tmp/ckan-mcp.skill`
470+
9. **Attach to release**: `gh release upload v0.x.0 ckan-mcp-server.dxt tmp/ckan-mcp.skill`
471+
10. **npm publish happens automatically**: pushing the tag in step 6 starts `.github/workflows/release.yml`, which verifies the tag matches `package.json`, builds, tests, and runs `npm publish --provenance`. **Do not run `npm publish` by hand** — the two paths collide and the loser gets `EPUBLISHCONFLICT`. Watch the run: `gh run watch $(gh run list --workflow=release.yml --limit 1 --json databaseId -q '.[0].databaseId')`
472+
11. **Publish to the MCP Registry** — only **after** the step 10 run has finished green, since the registry validates that the npm version exists: `mcp-publisher login github && mcp-publisher publish`. If login fails with `incorrect_device_code`, the local binary is stale: compare `mcp-publisher --version` against the [latest release](https://github.com/modelcontextprotocol/registry/releases) and update it — the device-auth flow has changed between versions, and the error does not say so
473+
12. **Deploy to Cloudflare** (if code changed): `npm run deploy`
473474

474475
See `docs/DEPLOYMENT.md` for detailed Cloudflare deployment instructions.
475476

476-
**Why steps 1 and 10 matter**: `server.json` feeds the official MCP Registry entry, which is what clients installing via the registry resolve. It is *not* updated by `npm publish`. Skipping it silently pins public installs to an old version: between v0.4.83 (2026-03-12) and v0.4.114 (2026-08-03) the registry advertised a build predating the v0.4.108 SSRF remediation, while npm was current. Verify after publishing:
477+
**Why steps 1 and 11 matter**: `server.json` feeds the official MCP Registry entry, which is what clients installing via the registry resolve. It is *not* updated by `npm publish`. Skipping it silently pins public installs to an old version: between v0.4.83 (2026-03-12) and v0.4.114 (2026-08-03) the registry advertised a build predating the v0.4.108 SSRF remediation, while npm was current. Verify after publishing:
477478

478479
```bash
479480
curl -s "https://registry.modelcontextprotocol.io/v0/servers?search=io.github.aborruso/ckan-mcp-server" | \

LOG.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,39 @@
22

33
## 2026-09-05
44

5+
### A release gate that asserts which dataset comes back
6+
7+
Three releases went out today, two of them to repair the one before. The verification in
8+
between was a sequence of commands rebuilt from memory each time, so it covered something
9+
different on each pass — and what it always covered was result counts. v0.4.122 shipped
10+
with counts verified and the ranking broken, because "22 results" and "the right dataset
11+
first" are different claims and only the second is what a caller asked for.
12+
13+
`npm run smoke` is that check as a command. Twelve known-answer cases from real telemetry
14+
in `tests/smoke/cases.json`, each asserting which dataset must come back and carrying the
15+
regression it guards; the runner starts the built server over HTTP, calls each tool the
16+
way a client does, and exits non-zero on the first failure.
17+
18+
Checked against the defects it claims to catch: reintroducing the v0.4.121 wrapping rule
19+
fails 4 of 12, and removing the parser probe from `ckan_find_relevant_datasets` — the
20+
v0.4.122 regression — fails the case requiring the two search tools to agree. Wired into
21+
the release workflow in `CLAUDE.md` (step 2, before the tag) and the checklist in
22+
`docs/DEPLOYMENT.md`.
23+
24+
Two things the gate needed on the way:
25+
26+
- the JSON format never exposed the query that actually ran, while Markdown has always
27+
shown it. `effective_query` now appears in `ckan_package_search` JSON output when the
28+
server rewrote the query, and is absent when it did not.
29+
- relevance scores summed unrounded fractions, printing totals like `8.299999999999999`.
30+
31+
Known and not fixed: the ranking model weights every query term equally, so on
32+
`defibrillatori Comune di Lecce` three datasets tie at 9.7 and the right one leads on
33+
Solr order rather than on score. Making the `tags` field proportional like the others was
34+
tried and reverted — it promotes "Elenco patrocini Comune di Lecce", whose tags carry two
35+
of the three terms against the defibrillator dataset's one. The fix is term specificity,
36+
a design change, not a patch.
37+
538
### v0.4.123 - relevance scoring and shared parser probe
639

740
Ships #536: field scoring by share of matched terms, Italian stopwords with acronyms preserved, Unicode-aware term matching, a wider candidate window, the parser probe shared with `ckan_find_relevant_datasets`, and accent-safe filters in `ckan_organization_search` and `ckan_tag_list`. `openspec/specs/ckan-search/spec.md` rewritten around the query-building path.

docs/DEPLOYMENT.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,10 @@ Verify:
651651
Use this checklist to ensure nothing is missed:
652652

653653
### Pre-Release
654+
- [ ] **Release gate green**: `npm run smoke` — known-answer search queries against live
655+
portals, asserting *which* dataset comes back rather than how many. v0.4.122 shipped
656+
with counts verified and ranking broken; "22 results" and "the right dataset first"
657+
are different claims, and only the second one is what a user asked for
654658
- [ ] Security audit clean: `npm audit` (0 vulnerabilities)
655659
- [ ] All tests passing: `npm test`
656660
- [ ] Code builds successfully: `npm run build`

docs/JSON-OUTPUT.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Error paths respect the requested format too: with `response_format: "json"` a f
3939
| Field | Type | Notes |
4040
|-------|------|-------|
4141
| `count` | number | Total matching datasets |
42+
| `effective_query` | string | Present **only when the server rewrote the query** — the caller sent `aria OR acqua` and Solr received `text:(aria OR acqua)`. Absent when the query ran unchanged, which is the common case. The Markdown format has always shown this as **Effective Query**. |
4243
| `results[].id` | string | Dataset UUID |
4344
| `results[].name` | string | Machine-readable slug |
4445
| `results[].title` | string | Human-readable title (falls back to name) |
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Expose the executed query in JSON search output
2+
3+
## Why
4+
5+
The server rewrites a caller's query before sending it to Solr: a boolean query on a
6+
portal whose default parser ignores booleans is wrapped as `text:(...)`, with its content
7+
escaped. The Markdown format has always reported this as **Effective Query**. The JSON
8+
format does not, so a JSON caller cannot tell whether the query it wrote is the query that
9+
ran.
10+
11+
That gap cost real time on 2026-09-05. Diagnosing why `bonifica siti contaminati Piemonte`
12+
returned 5 datasets instead of 22 meant reproducing calls by hand against the portal,
13+
because the tool's own JSON answer did not say which query it had executed. The release
14+
gate added in the same change (`npm run smoke`) cannot assert the parser behaviour without
15+
it either.
16+
17+
## What Changes
18+
19+
- `ckan_package_search` JSON output gains `effective_query`, present **only when the
20+
server rewrote the query** and absent when it ran unchanged.
21+
- No change to the Markdown format, which already carries the same information.
22+
- No change to any input, and no change to which datasets are returned.
23+
24+
## Impact
25+
26+
- Affected specs: `ckan-search`
27+
- Affected code: `compactSearchResult()` and its call site in `src/tools/package.ts`
28+
- Backwards compatible: the field is additive and optional. A client reading `count` and
29+
`results` is unaffected.
30+
- Documented in `docs/JSON-OUTPUT.md`.
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
## ADDED Requirements
2+
3+
### Requirement: Executed query visible in JSON output
4+
5+
When the server rewrites a caller's query before sending it to Solr, `ckan_package_search`
6+
SHALL report the executed query in its JSON output, so that a JSON caller can tell which
7+
query actually ran. The field SHALL be omitted when the query ran unchanged.
8+
9+
#### Scenario: Query rewritten
10+
- **WHEN** a JSON caller sends a query the server wraps in `text:(...)`
11+
- **THEN** the response carries `effective_query` with the query as executed
12+
13+
#### Scenario: Query unchanged
14+
- **WHEN** a JSON caller sends a query that reaches Solr as written
15+
- **THEN** the response carries no `effective_query`, and its shape is unchanged
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
## 1. Implementation
2+
3+
- [x] 1.1 Add the optional `effectiveQuery` parameter to `compactSearchResult()`
4+
- [x] 1.2 Pass it from the `ckan_package_search` JSON branch only when it differs from the
5+
caller's `q`
6+
- [x] 1.3 Document the field in `docs/JSON-OUTPUT.md`, stating that it is absent when the
7+
query ran unchanged
8+
9+
## 2. Verification
10+
11+
- [x] 2.1 `npm test` green
12+
- [x] 2.2 Release gate asserts both branches: `wrapped` on a boolean query where the portal
13+
needs it, `not_wrapped` on a plain query
14+
- [x] 2.3 Confirmed against a live portal that a plain query carries no `effective_query`
15+
and `aria OR acqua` on dati.comune.milano.it carries `text:(aria OR acqua)`

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@
2828
"pack:dxt": "rm -rf dxt-staging && mkdir -p dxt-staging/server && npm run build:dxt && echo '{\"type\":\"commonjs\"}' > dxt-staging/server/package.json && mkdir -p dxt-staging/server/node_modules && cp -r node_modules/undici dxt-staging/server/node_modules/ && cp manifest.json icon.png dxt-staging/ && dxt pack dxt-staging ckan-mcp-server.dxt && rm -rf dxt-staging",
2929
"pack:skill": "mkdir -p tmp && cd skills && zip -r ../tmp/ckan-mcp.skill ckan-mcp/ && cd ..",
3030
"prepack": "cp README.md .readme-full.md && cp .readme-npm.md README.md",
31-
"postpack": "mv .readme-full.md README.md"
31+
"postpack": "mv .readme-full.md README.md",
32+
"smoke": "npm run build && node scripts/smoke.mjs"
3233
},
3334
"keywords": [
3435
"mcp",

scripts/README.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,29 @@ schema change.
9595
bash scripts/worker_daily_stats.sh
9696
```
9797

98+
## smoke.mjs — the release gate
99+
100+
`npm run smoke` runs the known-answer cases in `tests/smoke/cases.json` against live
101+
portals and exits non-zero on the first failed assertion. `npm run smoke -- lecce` runs
102+
only the cases whose name matches.
103+
104+
Every case asserts **which** dataset comes back, not how many, and carries a `regression`
105+
field naming the failure it guards. That distinction is the reason the file exists:
106+
v0.4.122 shipped with result counts verified and the ranking broken, because a query
107+
returning 679 datasets and a query returning the right one first are different claims.
108+
109+
The gate is checked against the defects it claims to catch. Reintroducing the v0.4.121
110+
wrapping rule fails 4 of the 12 cases; removing the parser probe from
111+
`ckan_find_relevant_datasets`, the v0.4.122 regression, fails the case that requires the
112+
two search tools to agree.
113+
114+
Cases hit real portals, so the thresholds are loose enough to survive catalog drift and
115+
a failure can also mean a portal is down — check the message before assuming a code bug.
116+
117+
```bash
118+
npm run build && npm run smoke
119+
```
120+
98121
## GitHub Actions
99122

100123
`update-telemetry.yml` runs both scripts automatically twice a day (06:00 and 18:00 UTC).

0 commit comments

Comments
 (0)