feat(mcp/openapi): drive schemas from a programmatic Resource's static properties

[kylebernhardy]

Summary

Closes #1920 (follow-up to #1095/#1167). A programmatic Resource can declare static properties (a Record<string, JsonSchemaFragment>) instead of a table-backed attributes Array. The MCP tool builder, the OpenAPI generator, and the harper://schema introspection resource all read .attributes, so a bare static properties declaration produced skeletal schemas.

This adds projectPropertiesToAttributes / resolveAttributes (the structural inverse of the existing projectAttributesToProperties) and applies it on all three derivation surfaces, so a programmatic Resource gets the same rich schema a table-backed one does — types, per-property descriptions, enum/format/const, arrays, nested objects (incl. required/additionalProperties), and JSON-Schema union/nullable. Both type mappers (derive.ts, openApi.ts) pass lowercase JSON-Schema types through a single shared set (JSON_SCHEMA_SCALAR_TYPES), and existing table-backed output is unchanged.

Also replaces the previously ineffective idempotent-guard test (it asserted against a string map, so it passed trivially) with one that inspects the emitted tool's annotations, and adds MCP / OpenAPI / cross-surface convergence coverage that #1095 called for.

Where to look

• resources/jsonSchemaTypes.ts — fragmentToAttribute / projectPropertiesToAttributes / resolveAttributes. Round-trip is covered by a unit test asserting properties → attributes → properties is lossless (incl. enum/format/const, nested required/additionalProperties, arrays); a separate test covers ['T','null'] → nullable.
• resources/openApi.ts — the attribute loop handles nested objects and array-of-object (via attributeToFragment), JSON-scalar array elements, and attaches enum/format/const.
• components/mcp/tools/application.ts, components/mcp/resources.ts — one-line resolveAttributes swap each (cold path — registration / introspection, not per-request).

Attention

• Out of scope, tracked in [MCP] parsePath attribute-suffix routing doesn't resolve for programmatic static-properties Resources #1922: Resource.parsePath reads this.attributes for URL attribute-suffix routing (/Widget/id.label). A programmatic Resource with only static properties doesn't support that today — a pre-existing per-request-hot-path gap this PR doesn't touch (a fix wants a cached projection).
• Docs: [docs] Document programmatic Resource static properties for MCP/OpenAPI schema authoring #1923 tracks documenting the programmatic static properties authoring path (companion documentation-repo PR).
• Reviewed with /deep-review (api + data-integrity) and /code-review; the findings they surfaced — union/nullable collapse, OpenAPI array-of-object, dropped nested required/additionalProperties, mapper duplication — are fixed in the second commit (in history, not re-litigated here). The deep-review agents independently confirmed table-backed output is unchanged and permission filtering still applies.

PR generated by kAIle (Claude Opus 4.8).

[gemini-code-assist]

Code Review

This pull request introduces support for programmatic resources that declare static properties (in Record form) without an attributes array, ensuring consistent schema derivation across both MCP and OpenAPI surfaces. It adds utility functions to project JSON Schema properties back into attributes and includes comprehensive unit and convergence tests. The review feedback highlights two important improvements: guarding against a potential TypeError crash in OpenAPI generation when an array property omits items, and properly handling array-based nullable types (e.g., ['string', 'null']) during the reverse projection to preserve nullability and type resolution.

[kylebernhardy]

Docs companion opened: HarperFast/documentation#605 — docs(resources): document static properties as a first-class MCP/OpenAPI schema source. It documents the JSON Schema vocabulary, the full fragment-key list, and the union / item-less-array / required-on-create resolution rules this PR introduces.

Comment generated by kAIle (Claude Opus 4.8).

[kriszyp]

Could we avoid dropping every non-first union member here? static properties is typed as JSON Schema, and MCP accepts type arrays, so { type: ['string', 'number'] } currently becomes string-only; { type: ['null'] } becomes untyped. Please preserve the source union for MCP and explicitly translate it for OpenAPI 3.0 (nullable for T | null, oneOf for genuine unions), with a consumer-level regression test.

[kylebernhardy]

Partly addressed, partly not — splitting the two cases:

T | null is handled on the stacked follow-up #1944: MCP now re-expands to the source union ({ type: ['string','null'] }) rather than emitting a nullable keyword it has no use for, and OpenAPI emits nullable: true at top level and nested.

A genuine multi-type union (['string','number']) still collapses to its first member, on both PRs — the comment on this line admits it. Your oneOf suggestion is the right translation for OpenAPI, and MCP should simply pass the array through untouched since it accepts type arrays. That is not a small change to the projection: AttributeLike has a single type field, so preserving a union needs the attribute form to carry it, which is the same seam #1944 reworks. I would rather do it there or in a follow-up than partially here.

{ type: ['null'] } becoming untyped is a real edge of the same bug and I have no defense for it.

Comment generated by kAIle (Claude Opus 4.8)

[kylebernhardy]

Done properly now in 2039e4d — I stop deferring this to the follow-up.

AttributeLike carries the source union on a new types field, verbatim. type still holds the first non-null member so the single-type consumers (validation, query coercion) are untouched. Each surface then expresses it in its own dialect:

• MCP passes the array through as declared — it accepts type unions, as you said.
• OpenAPI 3.0 emits oneOf, your suggested translation. nullable rides alongside it for ["string","number","null"].
• { type: ["null"] } no longer goes untyped. On the stacked #1944 it emits { nullable: true, enum: [null] } — 3.0 has no null type, but a null-only enum is a form it can actually express, which beats staying silent about a field the author did describe.

The projection round-trips too: properties -> attributes -> properties returns the declared union rather than a collapsed scalar.

Consumer-level regression tests as you asked: oneOf emission, nullable-alongside-union, union round-trip, and a convergence test asserting both surfaces describe the same declaration without either narrowing it. On #1944 the translation lives in the shared emitter, so it applies at every nesting level rather than only the top — with a nested-union test to hold that.

Comment generated by kAIle (Claude Opus 4.8)

[kriszyp]

The projection turns type: ['string', 'null'] into type: 'string', nullable: true, but this attachment block never emits nullable. The final OpenAPI component therefore rejects null while MCP preserves it. Please emit nullable: true here and assert the generated component; nullable enums also need an emitted form that accepts null.

[kylebernhardy]

Agreed, and this one is already fixed on the stacked follow-up: #1944 emits nullable: true in exactly this attachment block (it was previously computed only to derive required and never written onto the property), with an assertion on the generated component in unitTests/components/mcp/tools/convergence.test.js plus a new emitted-document integration test.

The nullable-enum case you raise is not covered there — an enum whose list excludes null still rejects null even with nullable: true under 3.0. Tracking that with the dialect issue on your other thread.

Comment generated by kAIle (Claude Opus 4.8)

[kylebernhardy]

You were right and my earlier reply here was premature — the dialect commits fixed the nested and array paths but left this attachment block still dropping nullable, so the scalar case stayed broken. Fixed properly in f34cede:

• nullable: true is now emitted on the scalar schema when the attribute is nullable (covers both an author-declared nullable and a ["string","null"] union, which folds to the same attribute upstream).
• A nullable enum is widened with null, since 3.0's nullable does not widen an enum on its own.
• const now intersects with a co-declared enum rather than deferring to it, matching the nested path.

Tests assert the emitted component positively now (maybe, nullableEnum, nullableConst) instead of only checking that invalid keywords are absent — the old assertions would have passed just as happily with nullability dropped, which is why this survived.

Still open from your review and not addressed here: genuine multi-type unions (["string","number"]) still collapse to the first member rather than emitting oneOf. Leaving that for the convergence work in #1944 unless you want it in this PR.

Comment generated by kAIle (Claude Opus 4.8)

[kriszyp]

This recursion goes through attributeToFragment(), which intentionally omits enum/format/const; the scalar-array branch similarly rebuilds only type. As a result, nested fields and array items lose author-declared constraints (for example, items: { type: 'string', format: 'uuid', enum: [...] } becomes only { type: 'string' }). Could this use a recursive OpenAPI projector that retains supported hints at every depth, with nested/item assertions?

[kylebernhardy]

Confirmed, and also already fixed on the stacked follow-up #1944. It replaces this attributeToFragment() recursion with a shared, dialect-aware emitter (attributeToSchema) used by both the MCP deriver and the OpenAPI generator, so enum/format/const/description survive at every depth — nested objects and array items alike — and @hidden is suppressed at every level rather than only the top. Assertions cover nested and array-item hints on both surfaces.

The root cause was that attributeToFragment was serving two masters: the canonical, front-end-neutral Table.properties projection (which must omit those hints) and OpenAPI's nested emit (which must keep them).

Comment generated by kAIle (Claude Opus 4.8)

[kriszyp]

Because this document declares OpenAPI 3.0.3, const is not a valid Schema Object keyword (and lowercase type: 'null' passed through above is likewise not a 3.0 type). Please translate const to a single-value enum and nullability to OpenAPI 3.0 forms, or upgrade the whole document to 3.1. A spec-validation test would catch this dialect mismatch.

[kylebernhardy]

You are right, and this is the finding I had no answer for — thank you. The document declares 3.0.3, whose Schema Object is the draft-04 subset: const arrived in draft-06 and type: 'null' is not a 3.0 type (3.0 expresses nullability only via nullable). So we are emitting keywords the declared dialect does not define.

Worse, the stacked follow-up #1944 widens it — it carries const down into nested objects and array items too, and I documented const as a supported hint in the docs companion (documentation#605). All three need to change together.

Neither this PR nor #1944 addresses it, and I don't think it should be bolted onto either as a side fix — picking between "translate to 3.0 forms" (const → single-value enum, drop type: 'null' in favor of nullable, nullable-enum needs null appended to the list) and "declare 3.1" (which changes the contract for every existing consumer of the document, not just static-properties resources) is a real decision with downstream impact. Raising it with Kyle to scope as its own change, with the spec-validation test you suggest — that test is the thing that would have caught this and is worth having regardless of which direction we take.

Comment generated by kAIle (Claude Opus 4.8)

[kylebernhardy]

Fixed across all three artifacts, taking the conservative direction — translate to valid 3.0.3 forms rather than declaring 3.1, since upgrading the document changes the contract for every existing consumer, not just static-properties resources. If you'd rather go to 3.1, that's a separate decision and this doesn't foreclose it.

• harper#1921 (d10658161): const is emitted as the equivalent single-value enum; a bare type: 'null' becomes an untyped { nullable: true }, since 3.0 expresses nullability only via the keyword.
• harper#1944 (d00549032): the same translation inside the shared emitter, so it holds at depth — the dialect branch lives in attributeToSchema, with MCP keeping const (it speaks current JSON Schema) and OpenAPI getting enum.
• documentation#605 (cfdbab61): the reference and guide told authors const surfaces as written, which was never true. They now say: write const, expect enum: [value] in the OpenAPI output.

Your spec-validation suggestion is the part that earned its keep. I wrote it to walk the entire generated document for post-draft-04 keywords and null types rather than assert one property — and it immediately failed on #1944, catching six nested and array-item paths (components.schemas.Widget.properties.nested.properties.inner.const, …list.items.const, and the parameter copies) that my top-level fix had missed. Without that test I'd have shipped a half-fix and called it done.

Still outstanding from your other thread and not addressed here: nullable enums need null in the value list to actually accept null, since nullable: true alone doesn't extend an enum. Happy to fold that in if you want it on this PR, or leave it with the genuine-union oneOf work.

Comment generated by kAIle (Claude Opus 4.8).

[kriszyp]

Can we not use Object.assign?

[kylebernhardy]

Happy to drop it. For context on where it lives now: the stacked follow-up #1944 has one Object.assign, merging the per-surface primitive mapping into the fragment being built (Object.assign(fragment, options.mapPrimitive(attr.type, attr))). It is there because each surface's mapper returns a variable shape — MCP's Date yields { type: ['string','number'], description } and Bytes yields { type, contentEncoding }, so there is no fixed field list to copy.

If the objection is the opacity, I can replace it with explicit field copies for the keys we actually support, which also stops an unexpected key from a mapper leaking into the emitted schema. Say which you prefer and I will make the change on #1944.

Comment generated by kAIle (Claude Opus 4.8)

[kylebernhardy]

Dropped it — 61d2773 on #1944 copies the mapper result field by field instead.

Worth noting what that turned up: the wholesale merge was letting contentEncoding into emitted MCP schemas even though JsonSchemaFragment never declared it — the as JsonSchemaFragment cast on the mapper hid it. It is declared explicitly now. That is exactly the leak the explicit copy prevents, so this was the right call rather than just a style preference.

Comment generated by kAIle (Claude Opus 4.8)

[kriszyp]

Good. A few mechanical issues to work through.
_{🤖 Reviewed with GPT 5.6}

[kriszyp]

This shape-first branch runs before the union branch, so structural unions do not survive the projection. For example, { type: ['object', 'null'], properties: { x: { type: 'string' } } } takes this branch and loses both types and nullable; { type: ['array', 'null'], items: { type: 'string' } } takes the union branch later but never records elements, so it becomes an unconstrained array. Both MCP and OpenAPI then advertise a different contract from the declared JSON Schema. Please process the type union orthogonally to properties/items (and add round-trip cases for nullable object and array schemas) so both the structural shape and every union member survive.

[kriszyp]

For type: ['string', 'integer', 'null'], this creates an outer { oneOf: [...] }, and line 301 later adds nullable: true to that outer object. In OpenAPI 3.0.3, nullable only affects a type explicitly declared in the same Schema Object; this outer object has no type, so strict consumers still reject null. Please encode null in a branch that can actually admit it (for example, make exactly one typed oneOf member nullable) and validate the generated schema against a null instance in the test.

[kriszyp]

The recursive mapping bypasses attributeVisible, so hidden is enforced only at the top level. A declaration such as profile.properties.secret = { type: 'string', hidden: true, description: 'internal' } exposes secret and its description to MCP clients; if secret is in the nested required array, clients are also told they must send the hidden field. OpenAPI already filters this case. Please suppress hidden attributes at every recursive depth and prune suppressed names from the corresponding required list.

[kriszyp]

attributeToFragment deliberately omits enum, format, and const because it also feeds the front-end-neutral Table.properties projection. Using it as the consumer emitter therefore drops those constraints from every nested object; the type-only array branches likewise drop item-level hints. For example, a nested { type: 'string', enum: ['open', 'closed'] } or array item with const/format is retained by MCP but disappears from OpenAPI. Please use a consumer-facing recursive emitter (or propagate these hints explicitly at every depth) and add convergence assertions for nested and item-level hints.

[kriszyp]

Projected JSON-Schema properties normally have nullable === undefined, but deriveCreateSchema interprets !attr.nullable as required. Consequently { id: { primaryKey: true, type: 'string' }, label: { type: 'string' } } yields MCP required: ['label'], while this PR's OpenAPI path only adds a property when nullable === false and leaves label optional. MCP clients can reject otherwise valid create calls before dispatch. Please carry presence/requiredness explicitly through the projection, or otherwise align the undefined case across both surfaces without conflating optionality with nullability.

[kriszyp]

Although { type: 'array' } is valid general JSON Schema, OpenAPI 3.0.3 specifically requires items whenever type is array. Emitting the bare schema makes the generated 3.0.3 document invalid for strict tooling. Please translate an unconstrained array to { type: 'array', items: {} } (and do the same for array union members), then assert the OpenAPI-specific requirement rather than the source-dialect rule.

[kriszyp]

This unconditionally changes the intersection semantics of a standard JSON-Schema union. { type: ['string', 'null'], enum: ['a'] } rejects null because every keyword must match, but projection derives nullable: true from the type union and this line rewrites the enum to ['a', null]; the same issue occurs with const. Please preserve whether null came from the authored type union versus an explicit Harper nullable declaration, and do not add null to an enum/const that excluded it.