⚡️ Sanitize GQL Names in MCP Server Using `modules/types` Utils by mistryrn · Pull Request #1094 · overture-stack/arranger

mistryrn · 2026-08-26T13:55:41Z

Summary

Replaces duplicated functionality in the MCP Server with exports from the Arranger Types package, to standardize GraphQL name sanitization and avoid inconsistent behaviours.

🚨 NOTE: this exercise revealed a bug in the graphql-router module related to the changes that added support for unsupported GQL names. It has been flagged as a high severity tech debt item for now. Full details in the "GraphQL Router Module" section below.
UPDATE AUGUST 26, 2026 4PM: this bug has been resolved ✅

Issues

N/A

Description of Changes

MCP Server

Standardizes GQL name sanitization in the MCP Server by using utils imported from the modules/types package
- aggregation key --> sanitizeGraphqlFlatName
- hits selection args --> sanitizeGraphqlNameSegment, with parentPath still accumulating raw segments so fieldTypes lookups keep working
- documentType --> sanitized into a new rootFieldName on the returned request
Added @overture-stack/arranger-types as a dependency of apps/mcp-server

GraphQL Router Module

~~Flagged a high-severity bug in modules/graphql-router where the values returned for GQL identifiers that contained invalid characters (i.e. had to be sanitized) always come back as null~~
- ~~Filters work fine, since SQONs reach ES with raw names, which is why counts and filters look correct~~
- ~~Values for these fields always come back as null, because the resolvers are handing sanitized field names to ES, which has never heard of them~~

Dockerfile

Updated MCP Server targets in Dockerfiles to copy modules/types

Special Instructions

Before running these changes, run npm ci to make sure apps/mcp-server gets the new Arranger Types dependency:

# from root
npm ci

Readiness Checklist

Self Review
- I have performed a self review of code
- I have run the application locally and manually tested the feature
- I have checked all updates to correct typos and misspellings
Formatting
- Code follows the project style guide
- Autmated code formatters (ie. Prettier) have been run
Local Testing
- Successfully built all packages locally
- Successfully ran all test suites, all unit and integration tests pass
Updated Tests
- Unit and integration tests have been added that describe the bug that was fixed or the features that were added
Documentation
- All new environment variables added to .env.schema file and documented in the README
- All changes to server HTTP endpoints have open-api documentation
- All new functions exported from their module have TSDoc comment documentation

mistryrn

Adding some clarifying comments to help the review of this one 🙏

mistryrn · 2026-08-26T20:10:00Z

Now that the previously reported bug has been resolved, the changes in this file are mostly just prettier formatting. I'll highlight the one tech-debt entry that this PR resolved and removed.

mistryrn · 2026-08-26T20:11:11Z

-**Fix:** Either add `@overture-stack/arranger-types` as an mcp-server dependency and derive the set from `esToAggTypesMap`, or (preferred) expose each field's aggregation kind in the catalogue introspection response so MCP consumers need no local mapping at all. The latter aligns with the introspection-as-contract direction of the MCP integration readiness roadmap items.
-**Standalone:** yes; either fix is additive; no behaviour change for current field types
-
-### `execute_query` duplicates the raw-to-GraphQL-name transform and handles only dots


This tech-debt entry is resolved by this PR and is removed. The rest of the changes in this file are automatic formatting fixes from prettier.

mistryrn · 2026-08-26T20:13:37Z

+COPY --from=scaffolding --chown=$APP_USER:$APP_USER \
+	$APP_FOLDER/modules/types/dist \
+	./modules/types/dist
+COPY --from=scaffolding --chown=$APP_USER:$APP_USER \
+	$APP_FOLDER/modules/types/package.json \
+	./modules/types/package.json


Keeping the Dockerfiles in sync with the state of the application (now that modules/types is a dependency of apps/mcp-server)

mistryrn · 2026-08-26T20:15:08Z

+	// Catalogue B carries three fields whose raw ES names GraphQL disallows: `ca19-9_level`,
+	// `2020_baseline`, and the object path `donor-info.age-at-diagnosis`. The generated schema
+	// exposes them under sanitized names, so these tests pin the MCP server to the same rules the
+	// schema was built with. Before that, execute_query rejected all three outright.
+	//
+	// Both halves of the round trip are asserted: the sanitized names the response is keyed by,
+	// and the values behind them. The values matter as much as the keys, because the two are
+	// resolved by different code with different needs. This server sanitizes names on the way into
+	// the query document, while Arranger has to send Elasticsearch the *raw* names to read the
+	// values back. A regression in either direction is quiet, showing up as a `null` value or an
+	// empty aggregation under a key that still looks correct.
+	//
+	// Dataset reference (test/assets/catalogue_b.data.json):
+	//   b-001 ca19-9 12.5 low  age 47 | b-002 ca19-9 37.5 high age 61


New MCP Server integration tests added to cover cases with "awkward" field names 😉

mistryrn · 2026-08-26T20:20:12Z

+ * toDotNotationFieldName('donor__age_at_diagnosis') // returns 'donor.age_at_diagnosis'
 * ```
 */
-export const toAggregationFieldName = (fieldName: string): string => fieldName.split('.').join('__');


Main purpose of this PR: replace toAggregationFieldName with the types module's sanitizeGraphqlFlatName and sanitizeGraphqlNameSegment functions, to avoid further drift after Arranger added support for unsupported characters in GQL names.

* Standardizes GQL name sanitization in the MCP Server by using utils imported from the `modules/types` package * aggregation key --> `sanitizeGraphqlFlatName` * hits selection args --> `sanitizeGraphqlNameSegment`, with `parentPath` still accumulating raw segments so `fieldTypes` lookups keep working * `documentType` --> sanitized into a new `rootFieldName` on the returned request * Added `@overture-stack/arranger-types` as a dependency of `apps/mcp-server` * Updated MCP Server targets in Dockerfiles to copy `modules/types` * Flagged a high-severity bug in `modules/graphql-router` where the values returned for GQL identifiers that had to be sanitized (i.e. started with a number, or contained other invalid characters) always come back as `null`

* Restored the value assertions in `integration-tests/mcp-server`'s `execute_query` tests 18-21, now that the graphql-router resolvers send Elasticsearch raw field names. They shipped asserting only the sanitized response keys, because hits resolved to `null` regardless * Corrected four claims in `.dev/docs/build-sqon-implementation.md` written while the defect was live, including a section on the removed `toAggregationFieldName`. Annotated with dates rather than deleted * Documented the response-key convention in `CHANGELOG.md`: `execute_query` takes raw names in and returns results keyed by the sanitized ones, while names inside a `sqon` stay raw in both directions

mistryrn commented Aug 26, 2026

View reviewed changes

Comment thread .dev/tech-debt.md

Comment thread .dev/tech-debt.md Outdated

mistryrn force-pushed the feat/1080-build-sqon-v2 branch from 2745143 to 1d66e30 Compare August 26, 2026 19:55

mistryrn force-pushed the fix/mcp-sanitize-gql-names-with-types-module branch from 277cb67 to 375ff2c Compare August 26, 2026 20:06

mistryrn commented Aug 26, 2026

View reviewed changes

mistryrn marked this pull request as ready for review August 26, 2026 20:29

mistryrn requested a review from justincorrigible August 28, 2026 13:07

Base automatically changed from feat/1080-build-sqon-v2 to main August 28, 2026 13:36

mistryrn added 2 commits August 28, 2026 09:43

mistryrn force-pushed the fix/mcp-sanitize-gql-names-with-types-module branch from 375ff2c to f4489d9 Compare August 28, 2026 13:52

justincorrigible merged commit 4b52515 into main Aug 28, 2026
2 checks passed

justincorrigible deleted the fix/mcp-sanitize-gql-names-with-types-module branch August 28, 2026 17:06

Navigation Menu

Uh oh!

⚡️ Sanitize GQL Names in MCP Server Using `modules/types` Utils - #1094

⚡️ Sanitize GQL Names in MCP Server Using `modules/types` Utils#1094
justincorrigible merged 2 commits into
mainfrom
fix/mcp-sanitize-gql-names-with-types-module

mistryrn commented Aug 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

mistryrn left a comment

Uh oh!

mistryrn Aug 26, 2026

Uh oh!

mistryrn Aug 26, 2026

Uh oh!

mistryrn Aug 26, 2026

Uh oh!

mistryrn Aug 26, 2026

Uh oh!

mistryrn Aug 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mistryrn commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issues

Description of Changes

MCP Server

GraphQL Router Module

Dockerfile

Special Instructions

Readiness Checklist

Uh oh!

Uh oh!

Uh oh!

mistryrn left a comment

Choose a reason for hiding this comment

Uh oh!

mistryrn Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

mistryrn Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

mistryrn Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

mistryrn Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

mistryrn Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mistryrn commented Aug 26, 2026 •

edited

Loading