fix(mcp/openapi): share one schema emitter so the two surfaces stop diverging

[kylebernhardy]

Closes #1941, closes #1942.

Stacked on #1921. The base branch is feat/mcp-static-properties-tests-1920, not main — the code paths these issues describe (OpenAPI's nested-object branch, the enum/format/const passthrough, JSON_SCHEMA_SCALAR_TYPES) only exist there. GitHub will retarget this to main automatically when #1921 merges. Do not merge before #1921.

Root cause

attributeToFragment ended up serving two masters: the canonical, front-end-neutral Table.properties Record, and — since #1921 — OpenAPI's nested-object emit. Those have opposite requirements. The canonical projection must carry hidden (it round-trips through fragmentToAttribute) and must not carry enum/format/const (a code-first types.enum column has to stay identical to its GraphQL equivalent). An emitted document is the exact reverse. So the nested paths quietly drifted away from MCP's, and every divergence in #1942 lives at that seam.

Rather than bolt more conditionals onto the shared projector, this adds a second, purpose-built one:

attributeToSchema(attr, { dialect, mapPrimitive })

One traversal — nesting, arrays, hidden suppression, hint propagation, nullability — used by both surfaces. Each surface keeps its own primitive mapping, because those differences are legitimate: MCP widens Date to ['string','number'] and tags Bytes with contentEncoding; OpenAPI carries the Harper type name as format. Unifying those too would churn table-backed output, which #1921 was careful to leave alone. attributeToFragment is untouched.

What changed behaviorally

#1941 — hidden is honored at every nesting level, not just the top. A hidden sub-property of a nested object was previously emitted on both surfaces with its description, and OpenAPI additionally wrote hidden: true into the public document as a schema key — advertising that the field was meant to be suppressed. Harper directives (hidden, primaryKey, assignCreatedTime, assignUpdatedTime) are no longer emitted into schemas at all. A suppressed name is also dropped from the nested required list, which would otherwise make the object unsatisfiable for a validating client.

#1942 — all three:

• An unrecognized type name ('Text') coerced to string on MCP and to untyped {} on OpenAPI. Both now emit untyped, and resolveDeclaredType warns once per name so the typo is visible rather than silently producing two different wrong schemas.
• enum/format/const/description reached only top-level properties on OpenAPI. Nested objects and array items now carry them, matching MCP.
• OpenAPI read nullable only to compute required and never emitted it — so a nullable top-level property was advertised as non-nullable, while a nested one (routed through the shared projector) was not. Both levels now emit nullable: true; MCP continues to emit the ['T','null'] union and never the keyword.

Where to look

• applyNullability is the one place the dialects legitimately differ, and it's the piece most worth checking. OpenAPI 3.0.3 has no union types, so nullable: true is the only expression available; JSON Schema has no nullable keyword. If you'd rather MCP also emit nullable for client compatibility, that's a one-line change — but it would be non-standard.
• Two of my own tests failed on first run and caught real gaps: OpenAPI's primitive array-item branch still bypassed the shared emitter (dropping item enum/description), and I'd added resolveDeclaredType without wiring it into MCP's default: case. Both fixed; the tests are the reason I know.
• The emitted-output change is user-visible. Anyone consuming /openapi.json sees nullable: true appear on top-level properties, hidden: true disappear from nested ones, and untyped {} where an unrecognized type name previously produced {"type":"string"} on the MCP side. That last one is the only case where a client could see less type information than before — deliberate, since the previous value was a guess.

Seven convergence tests assert MCP and OpenAPI output side by side for the same fragment, which is the regression shape these issues kept slipping through. 534 tests pass across the MCP, OpenAPI, GraphQL-metadata, and code-first-parity suites.

Docs follow-up: documentation#605 documents the current (pre-fix) per-surface caveats and will need them collapsed once this lands.

PR description generated by kAIle (Claude Opus 4.8).

[gemini-code-assist]

Code Review

This pull request unifies and refactors the schema generation logic for MCP tools and OpenAPI 3.0.3 endpoints by introducing a shared attributeToSchema function in resources/jsonSchemaTypes.ts. This ensures consistent traversal, nesting, array handling, hidden property suppression, and nullability across both surfaces, addressing previous divergence issues. Feedback focuses on allowing user-defined hints to overwrite generic defaults from primitive mappings, passing the current attribute to the mapPrimitive callback to improve warning context accuracy, and omitting the required array if it becomes empty after filtering out hidden properties.

[gemini-code-assist]

In attributeToSchema, user-defined hints (like description, enum, format, and const) are only applied if they are currently undefined on the fragment. However, primitive mappings (such as Date in harperTypeToJsonSchema) can provide generic default descriptions (e.g., 'ISO 8601 timestamp or epoch milliseconds.'). If a user defines a custom description on a Date attribute, it will be silently ignored because fragment.description is already populated by the primitive mapping.

Additionally, this creates a divergence with top-level OpenAPI properties where the user's description always overwrites the default.

We should let the user-defined attributes directly overwrite any generic defaults from the primitive mapping.

Suggested change

	if (attr.description && fragment.description === undefined) fragment.description = attr.description;
	if (attr.enum && fragment.enum === undefined) fragment.enum = attr.enum;
	if (attr.format && fragment.format === undefined) fragment.format = attr.format;
	if (attr.const !== undefined && fragment.const === undefined) fragment.const = attr.const;
	if (attr.description) fragment.description = attr.description;
	if (attr.enum) fragment.enum = attr.enum;
	if (attr.format) fragment.format = attr.format;
	if (attr.const !== undefined) fragment.const = attr.const;

[gemini-code-assist]

The mapPrimitive callback currently only receives the type string. When resolving types for nested properties, this makes it impossible to know which specific property is being mapped, leading to warning messages that incorrectly attribute the unrecognized type to the top-level property name.

Let's update the signature of mapPrimitive to also accept the current AttributeLike object so that callers can use the correct property name for warning contexts.

Suggested change

	mapPrimitive: (type: string | undefined) => JsonSchemaFragment;
	mapPrimitive: (type: string | undefined, attr: AttributeLike) => JsonSchemaFragment;

[gemini-code-assist]

Pass the current attr to mapPrimitive to support the signature change described in the main finding on SchemaEmitOptions.

Suggested change

	Object.assign(fragment, options.mapPrimitive(attr.type));
	Object.assign(fragment, options.mapPrimitive(attr.type, attr));

[gemini-code-assist]

Update the callback signature to accept leafAttr to support the signature change described in the main finding on SchemaEmitOptions.

Suggested change

	mapPrimitive: (type) => harperTypeToJsonSchema(type) as JsonSchemaFragment,
	mapPrimitive: (type, leafAttr) => harperTypeToJsonSchema(type) as JsonSchemaFragment,

[gemini-code-assist]

Use the leafAttr parameter to resolve the correct nested property name in the warning context, as described in the main finding on SchemaEmitOptions.

Suggested change

	mapPrimitive: (type) => {
	if (type === 'Any' || type === undefined) return {};
	const resolved = resolveDeclaredType(type, `OpenAPI property "${attr.name}"`);
	if (!resolved) return {};
	// Preserve the Harper type name as `format` for the types where it adds information, matching
	// the top-level `Type()` emitter.
	return DATA_TYPES[type] ? (new Type(resolved, type) as JsonSchemaFragment) : { type: resolved };
	},
	mapPrimitive: (type, leafAttr) => {
	if (type === 'Any' || type === undefined) return {};
	const resolved = resolveDeclaredType(type, `OpenAPI property "${leafAttr.name}"`);
	if (!resolved) return {};
	// Preserve the Harper type name as `format` for the types where it adds information, matching
	// the top-level `Type()` emitter.
	return DATA_TYPES[type] ? (new Type(resolved, type) as JsonSchemaFragment) : { type: resolved };
	},

[gemini-code-assist]

If all required properties of a nested object are hidden, fragment.required will be filtered down to an empty array. While technically valid, emitting an empty required array is redundant and can be omitted entirely for cleaner schema output.

Suggested change

	if (attr.required) fragment.required = attr.required.filter((name) => visibleNames.has(name));
	if (attr.required) {
	const filtered = attr.required.filter((name) => visibleNames.has(name));
	if (filtered.length > 0) fragment.required = filtered;
	}

[claude]

1 blocker: fragmentToOpenApiSchema (resources/openApi.ts:672-708) narrows a genuine multi-type union to its first member instead of oneOf, for the request-contract body/response/query-param path — the same bug class this PR fixes everywhere else. See inline comment.

[claude]

1. Primitive-mapping defaults silently block user-defined hints (MCP Date description)

File: resources/jsonSchemaTypes.ts:309-312

What: attr.description/enum/format/const only overwrite the fragment when the corresponding field is still undefined. But mapPrimitive runs first (line 305) and can populate fragment.description itself — MCP's harperTypeToJsonSchema does exactly this for Date, hardcoding 'ISO 8601 timestamp or epoch milliseconds.'. If an attribute of type Date also declares its own description, that user-authored text is silently dropped in favor of the generic default, because fragment.description is already non-undefined by the time line 309 runs.

Why it matters: This is the exact class of MCP/OpenAPI divergence this PR sets out to close (#1941/#1942) — OpenAPI's mapPrimitive never sets description, so a Date attribute's custom description survives on OpenAPI but is silently discarded on MCP. It's also a real regression for integrators: a documented field description is dropped without any warning. This was flagged by a peer reviewer (gemini) on the first commit and was not addressed by the 7281aaa follow-up, which fixed several sibling gemini findings (mapPrimitive signature, empty required) but not this one. None of the added convergence tests exercise a Date-typed attribute with a custom description, so the gap isn't caught.

Suggested fix: Let the attribute-level hint always win over a primitive-mapping default, as gemini suggested:

Suggested change

	if (attr.nullable) applyNullability(fragment, options.dialect);
	if (attr.description && fragment.description === undefined) fragment.description = attr.description;
	if (attr.enum && fragment.enum === undefined) fragment.enum = attr.enum;
	if (attr.format && fragment.format === undefined) fragment.format = attr.format;
	if (attr.const !== undefined && fragment.const === undefined) fragment.const = attr.const;
	if (attr.description) fragment.description = attr.description;
	if (attr.enum) fragment.enum = attr.enum;
	if (attr.format) fragment.format = attr.format;
	if (attr.const !== undefined) fragment.const = attr.const;

[claude]

Still open from the prior review round (flagged here by both gemini and this bot, and not addressed by the 7281aaa5d/f54031bda follow-ups): mapPrimitive runs first and can pre-populate fragment.description (MCP's harperTypeToJsonSchema hardcodes 'ISO 8601 timestamp or epoch milliseconds.' for Date). Because these lines only assign when the field is still undefined, a user-authored description on a Date attribute is silently dropped on MCP — while it survives on OpenAPI, since OpenAPI's mapPrimitive never sets description. That's the same divergence class #1941/#1942 set out to close, and no test in this PR's convergence suite exercises a Date attribute with a custom description, so it isn't caught.

Suggested change

	if (attr.description && fragment.description === undefined) fragment.description = attr.description;
	if (attr.enum && fragment.enum === undefined) fragment.enum = attr.enum;
	if (attr.format && fragment.format === undefined) fragment.format = attr.format;
	if (attr.const !== undefined && fragment.const === undefined) fragment.const = attr.const;
	if (attr.description) fragment.description = attr.description;
	if (attr.enum) fragment.enum = attr.enum;
	if (attr.format) fragment.format = attr.format;
	if (attr.const !== undefined) fragment.const = attr.const;

[claude]

Still open (re-raising from the prior thread — flagged by both gemini and this bot, addressed for const/dialect handling in the two latest commits but not for this): mapPrimitive runs first and MCP's Date mapping pre-populates fragment.description, so these === undefined guards mean a user-authored description on a Date attribute is silently dropped on MCP while it survives on OpenAPI. Suggested fix unchanged:

Suggested change

	if (attr.description && fragment.description === undefined) fragment.description = attr.description;
	if (attr.enum && fragment.enum === undefined) fragment.enum = attr.enum;
	if (attr.format && fragment.format === undefined) fragment.format = attr.format;
	if (attr.const !== undefined) {
	if (attr.description) fragment.description = attr.description;
	if (attr.enum) fragment.enum = attr.enum;
	if (attr.format) fragment.format = attr.format;

[claude]

What: fragmentToOpenApiSchema — the function that emits request-contract bodies/responses/query params (resources/openApi.ts:672-708) — narrows a genuine multi-type union to only its first member. When fragment.type is e.g. ['string', 'number'] (no 'null'), members is ['string','number'], and line 682 sets schema.type = members[0], silently dropping 'number'.

Why it matters: This is the exact bug class this PR's own later commits (2039e4dec "preserve a declared type union instead of keeping its first member", 61d2773fa) fix everywhere else — the shared attributeToSchema/toOpenApiDialect path now converts a genuine union to oneOf (see resources/jsonSchemaTypes.ts:334-338 and the top-level loop's union/oneOf branch in this same file). fragmentToOpenApiSchema feeds every request-contract body, response, and query parameter (defineResource/Resource.withSchema), and defineResource.ts itself validates against a type array at runtime (Array.isArray(fragment.type) ? fragment.type : [fragment.type], line ~301) — so a contract that legitimately accepts string | number will have its OpenAPI doc claim only string is accepted. That's integrator-misleading documentation drift, and it diverges from what the server actually enforces on the same input.

The added test (translates request-contract schemas too) only exercises note: { type: ['string', 'null'] }, which is nullability, not a genuine union — it wouldn't catch this.

Suggested fix: mirror the oneOf translation used by the table path:

Suggested change

	if (Array.isArray(fragment.type)) {
	const members = fragment.type.filter((t: unknown) => t !== 'null');
	if (members.length !== fragment.type.length) schema.nullable = true;
	if (members.length > 0) schema.type = members[0];
	if (Array.isArray(fragment.type)) {
	const members = fragment.type.filter((t: unknown) => t !== 'null');
	if (members.length !== fragment.type.length) schema.nullable = true;
	if (members.length > 1) schema.oneOf = members.map((member: unknown) => ({ type: member }));
	else if (members.length === 1) schema.type = members[0];
	} else if (fragment.type != null && fragment.type !== 'null') {
	schema.type = fragment.type;
	}

[kriszyp]

A few more things to cover.
_{🤖 Reviewed with GPT 5.6}

[kriszyp]

This remains a second schema emitter, and it narrows every genuine union to members[0]. For example, a request-contract property declared as schemaOf({ type: ['string', 'number'] }) is accepted as either member by runtime validation and exposed as either member to MCP, but OpenAPI advertises only string; generated clients can therefore reject valid numeric requests/responses. Please route request-contract fragments through the shared dialect-aware emitter (or extract one fragment-level emitter used by both paths), and add body/response/query coverage for a non-null multi-type union.

[kriszyp]

OpenAPI 3.0.3 says nullable only takes effect when type is explicitly defined in the same Schema Object (https://spec.openapis.org/oas/v3.0.3.html#fixed-fields-20). A declaration such as type: ['string', 'integer', 'null'] therefore becomes two non-null oneOf branches plus an inert nullable: true, so OpenAPI clients still reject null while MCP accepts it. Please encode null as an explicit union branch (for example { enum: [null] }) and apply the same fix to the duplicated top-level OpenAPI path, with a validation test that actually checks a null instance.

[kriszyp]

Although { type: 'array' } is valid general JSON Schema, OpenAPI 3.0.3 requires items whenever type is array (https://spec.openapis.org/oas/v3.0.3.html#schema-object). This branch emits a bare array, and the shared nested emitter/request-contract translator do the same, so a supported static declaration can make the entire OpenAPI document fail strict validation. For the OpenAPI dialect, please emit items: {} for an unconstrained array (MCP can keep the bare form) and cover top-level, nested, and request-contract arrays.

[kriszyp]

JsonSchemaFragment is the author-facing IR behind SchemaSource and Resource.properties/inputSchemas/outputSchemas, so adding oneOf here makes schemaOf({ oneOf: [...] }) a supported TypeScript declaration. Runtime validateValue, however, treats that fragment as untyped because it has no type, and the request-contract OpenAPI translator drops oneOf; an apparently constrained body is therefore accepted without the advertised validation. Please either keep dialect-only output fields in a separate emitted-schema type, or implement authored oneOf end-to-end before exposing it in this public shape.