From 7ba6f91d9ced6bb3e7c7c45b6b63a00ad79d5df7 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 16:04:46 -0600 Subject: [PATCH 01/10] feat(mcp/openapi): drive schemas from a programmatic Resource's static properties (#1920) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A programmatic Resource may declare `static properties` (a Record) without an `attributes` Array. The MCP tool builder, the OpenAPI generator, and the harper://schema introspection resource all read `.attributes`, so such a Resource produced skeletal schemas. Add `projectPropertiesToAttributes` / `resolveAttributes` (the structural inverse of `projectAttributesToProperties`) and use it on all three derivation surfaces, so a bare `static properties` declaration yields the same rich schema — types, per-property descriptions, enum/format/const, arrays, and nested objects — a table-backed Resource gets. Both type mappers (derive.ts, openApi.ts) pass through lowercase JSON-Schema types (static properties speaks JSON Schema; Harper's GraphQL types are capitalized, so there is no collision) and existing table-backed output is unchanged. Also fixes the previously ineffective idempotent-guard test (it checked a string map, not an emitted tool's annotations) and adds MCP, OpenAPI, and cross-surface convergence coverage. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/resources.ts | 5 +- components/mcp/tools/application.ts | 5 +- components/mcp/tools/schemas/derive.ts | 17 +++ resources/jsonSchemaTypes.ts | 59 +++++++++ resources/openApi.ts | 39 ++++-- .../components/mcp/tools/application.test.js | 124 ++++++++++++++++++ .../components/mcp/tools/convergence.test.js | 77 +++++++++++ .../components/mcp/tools/operations.test.js | 31 ++++- unitTests/resources/graphqlMetadata.test.js | 33 +++++ unitTests/resources/openApi.test.js | 54 ++++++++ 10 files changed, 428 insertions(+), 16 deletions(-) create mode 100644 unitTests/components/mcp/tools/convergence.test.js diff --git a/components/mcp/resources.ts b/components/mcp/resources.ts index 37cb0109ef..601630039f 100644 --- a/components/mcp/resources.ts +++ b/components/mcp/resources.ts @@ -38,6 +38,7 @@ import harperLogger from '../../utility/logging/harper_logger.ts'; import { AccessViolation } from '../../utility/errors/hdbError.ts'; import { SERVER_CAPABILITIES, SERVER_INFO, SUPPORTED_PROTOCOL_VERSIONS } from './lifecycle.ts'; import { encodeCursor } from './pagination.ts'; +import { resolveAttributes } from '../../resources/jsonSchemaTypes.ts'; import { customResourceCompletionValues, listCustomResources, @@ -820,7 +821,9 @@ function readTableSchema(db: string, table: string, user: AuthedUser, href: stri } } if (!resource) return { ok: false, reason: `table not found: ${db}.${table}` }; - const attributes = resource.attributes ?? []; + // Fall back to a programmatic Resource's `static properties` when it declares no attributes Array, + // so schema introspection matches the tool/OpenAPI surfaces. + const attributes = resolveAttributes(resource); const filteredAttributes = filterAttributesByPermissions(attributes, perm?.attribute_permissions); const body = { database: db, diff --git a/components/mcp/tools/application.ts b/components/mcp/tools/application.ts index bff12a8ec9..8e0698d22a 100644 --- a/components/mcp/tools/application.ts +++ b/components/mcp/tools/application.ts @@ -49,6 +49,7 @@ import { } from '../customResourceRegistry.ts'; import { notifyPromptsListChanged, notifyResourcesListChanged, notifyToolsListChanged } from '../listChanged.ts'; import { decodeCursor, encodeCursor } from '../pagination.ts'; +import { resolveAttributes } from '../../../resources/jsonSchemaTypes.ts'; import { type AttributePermissionEntry, type HarperAttribute, @@ -1356,7 +1357,9 @@ function buildApplicationTools(resources: ResourcesRegistry): void { const tableName = ResourceClass?.tableName; const suffix = uniqueSuffix(path, databaseName, claimedSuffixes); claimedSuffixes.add(suffix); - const attributes = (ResourceClass?.attributes ?? []) as HarperAttribute[]; + // A programmatic Resource may declare `static properties` without an `attributes` Array; resolve + // the effective attributes so its verb tools get a rich inputSchema instead of a skeletal one. + const attributes = resolveAttributes(ResourceClass) as HarperAttribute[]; if (hasVerbs) { toolsRegistered += registerVerbTools({ path, diff --git a/components/mcp/tools/schemas/derive.ts b/components/mcp/tools/schemas/derive.ts index 97a04199cf..984d159f50 100644 --- a/components/mcp/tools/schemas/derive.ts +++ b/components/mcp/tools/schemas/derive.ts @@ -25,6 +25,10 @@ export interface HarperAttribute { assignCreatedTime?: boolean; assignUpdatedTime?: boolean; expiresAt?: boolean; + // JSON-Schema hints a programmatic Resource may carry via `static properties`. + enum?: readonly (string | number | boolean | null)[]; + format?: string; + const?: unknown; } export interface AttributePermissionEntry { @@ -63,6 +67,16 @@ function harperTypeToJsonSchema(type: string | undefined): { type: string | stri case 'Any': case undefined: return {}; + case 'string': + case 'integer': + case 'number': + case 'boolean': + case 'object': + case 'array': + case 'null': + // A programmatic Resource's `static properties` already speaks JSON Schema (lowercase types, + // no collision with Harper's capitalized GraphQL types); pass those through unchanged. + return { type }; default: return { type: 'string' }; } @@ -97,6 +111,9 @@ function attributeToProperty(attr: HarperAttribute): object { if (attr.description && !base.description) { base.description = attr.description; } + if (attr.enum && !('enum' in base)) base.enum = attr.enum; + if (attr.format && !('format' in base)) base.format = attr.format; + if (attr.const !== undefined && !('const' in base)) base.const = attr.const; return base; } diff --git a/resources/jsonSchemaTypes.ts b/resources/jsonSchemaTypes.ts index e6a539be70..779388fa31 100644 --- a/resources/jsonSchemaTypes.ts +++ b/resources/jsonSchemaTypes.ts @@ -59,6 +59,11 @@ export interface AttributeLike { elements?: AttributeLike; /** Sub-attributes of a nested object field (the same array form `Table.validate` iterates). */ properties?: AttributeLike[]; + // JSON-Schema-only hints an author may declare on `static properties`; carried through the + // projection so they survive the properties <-> attributes round-trip. + enum?: readonly (string | number | boolean | null)[]; + format?: string; + const?: unknown; } /** @@ -93,6 +98,9 @@ export function attributeToFragment(attr: AttributeLike): JsonSchemaFragment { if (attr.assignUpdatedTime) fragment.assignUpdatedTime = true; if (attr.hidden) fragment.hidden = true; if (attr.nullable) fragment.nullable = true; + if (attr.enum) fragment.enum = attr.enum; + if (attr.format) fragment.format = attr.format; + if (attr.const !== undefined) fragment.const = attr.const; return fragment; } @@ -108,3 +116,54 @@ export function projectAttributesToProperties(attributes: AttributeLike[]): Reco } return result; } + +/** + * Structural inverse of `attributeToFragment`: rebuild an attribute from a JSON Schema fragment. + * A programmatic Resource may declare `static properties` (the Record form) without populating the + * `attributes` Array; the schema-derivation paths (MCP `derive.ts`, OpenAPI) read attributes, so a + * bare declaration would otherwise yield a skeletal schema. Projecting the fragments back into + * attributes lets those paths produce the same rich schema they build for table-backed resources. + */ +function fragmentToAttribute(name: string, fragment: JsonSchemaFragment): AttributeLike { + const attr: AttributeLike = { name }; + if (fragment.properties) { + attr.properties = Object.entries(fragment.properties).map(([subName, sub]) => fragmentToAttribute(subName, sub)); + } else if (fragment.type === 'array' && fragment.items) { + attr.type = 'array'; + attr.elements = fragmentToAttribute(name, fragment.items); + } else if (fragment.type != null) { + attr.type = Array.isArray(fragment.type) ? fragment.type[0] : fragment.type; + } + if (fragment.description) attr.description = fragment.description; + if (fragment.primaryKey) attr.isPrimaryKey = true; + if (fragment.assignCreatedTime) attr.assignCreatedTime = true; + if (fragment.assignUpdatedTime) attr.assignUpdatedTime = true; + if (fragment.hidden) attr.hidden = true; + if (fragment.nullable) attr.nullable = true; + if (fragment.enum) attr.enum = fragment.enum; + if (fragment.format) attr.format = fragment.format; + if (fragment.const !== undefined) attr.const = fragment.const; + return attr; +} + +/** + * Project a `Record` (the `static properties` form) back into the + * `Attribute[]` Array the schema-derivation paths consume. Inverse of `projectAttributesToProperties`. + */ +export function projectPropertiesToAttributes(properties: Record): AttributeLike[] { + return Object.entries(properties).map(([name, fragment]) => fragmentToAttribute(name, fragment)); +} + +/** + * The effective attribute Array for a Resource/Table class: its declared `attributes` when present, + * otherwise the projection of a bare `static properties` declaration. Keeps MCP and OpenAPI schema + * derivation identical for table-backed and programmatic Resources. + */ +export function resolveAttributes(source?: { + attributes?: AttributeLike[]; + properties?: Record; +}): AttributeLike[] { + if (source?.attributes?.length) return source.attributes; + if (source?.properties) return projectPropertiesToAttributes(source.properties); + return []; +} diff --git a/resources/openApi.ts b/resources/openApi.ts index f9480416bb..bce5f80690 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -1,10 +1,15 @@ import { packageJson } from '../utility/packageUtils.js'; import { Resources, routePatternToTemplate } from './Resources.ts'; import { Resource } from './Resource.ts'; -import { DATA_TYPES } from './jsonSchemaTypes.ts'; +import { DATA_TYPES, attributeToFragment, projectPropertiesToAttributes } from './jsonSchemaTypes.ts'; const OPENAPI_VERSION = '3.0.3'; +// A programmatic Resource's `static properties` uses JSON Schema types directly (lowercase). Harper's +// GraphQL attribute types are all capitalized and live in DATA_TYPES, so a lowercase scalar reaching +// the attribute mapper came from `static properties` and should be emitted as-is. +const JSON_SCHEMA_SCALARS = new Set(['string', 'integer', 'number', 'boolean', 'object', 'array', 'null']); + const SCHEMA_COMP_REF = '#/components/schemas/'; const DESCRIPTION_200 = 'successful operation'; @@ -115,10 +120,15 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { // Used as both the schema-level `description` (in components.schemas) and as a prefix // on each path-level operation description. const tableDoc: string | undefined = resource.Resource.description; - if (!attributes && resources.allTypes.has(resource.path)) { + // A programmatic Resource may declare `static properties` (Record) without an `attributes` + // Array; project it so per-property schemas are emitted instead of a skeletal object. + if (!attributes?.length && resource.Resource.properties) { + attributes = projectPropertiesToAttributes(resource.Resource.properties); + } + if (!attributes?.length && resources.allTypes.has(resource.path)) { const possibleType = resources.allTypes.get(resource.path); sealed = possibleType.sealed; - attributes = possibleType.attributes ?? possibleType.properties; + attributes = possibleType.attributes ?? projectPropertiesToAttributes(possibleType.properties ?? {}); } if (!primaryKey) continue; const props = {}; @@ -151,21 +161,34 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { } else { props[name] = { $ref: SCHEMA_COMP_REF + def.type }; } + } else if (attr.properties) { + // Nested object from `static properties` — the shared projector emits the full object + // schema (sub-properties recursed); OpenAPI's table path uses $refs instead. + props[name] = attributeToFragment(attr); } else if (type === 'array') { if (elements.type === 'Any') { props[name] = { type: 'array', items: { format: elements.type } }; + } else if (!DATA_TYPES[elements.type] && JSON_SCHEMA_SCALARS.has(elements.type)) { + props[name] = { type: 'array', items: new Type(elements.type) }; } else { props[name] = { type: 'array', items: new Type(DATA_TYPES[elements.type], elements.type) }; } } else if (type === 'Any') { props[name] = { format: type }; + } else if (!DATA_TYPES[type] && JSON_SCHEMA_SCALARS.has(type)) { + props[name] = new Type(type); } else { props[name] = new Type(DATA_TYPES[type], type); } } - // Attach per-property description so it surfaces in Swagger UI / Redoc. - if (description && props[name] && typeof props[name] === 'object' && !('$ref' in props[name])) { - (props[name] as { description?: string }).description = description; + // Attach per-property JSON-Schema hints (description/enum/format/const) so they surface in + // Swagger UI / Redoc; enum in particular tells clients the allowed values. + if (props[name] && typeof props[name] === 'object' && !('$ref' in props[name])) { + const prop = props[name] as { description?: string; enum?: unknown; format?: string; const?: unknown }; + if (description) prop.description = description; + if (attr.enum && prop.enum === undefined) prop.enum = attr.enum; + if (attr.format && prop.format === undefined) prop.format = attr.format; + if (attr.const !== undefined && prop.const === undefined) prop.const = attr.const; } queryParamsArray.push(new Parameter(name, 'query', props[name])); } @@ -542,10 +565,10 @@ function ResourceSchema(properties, additionalProperties?: boolean, required?: s if (description) this.description = description; } -function Type(type, format) { +function Type(type, format?) { this.type = type; if (type === 'string' || type === 'number' || type === 'integer') { - if (format !== 'String') { + if (format !== undefined && format !== 'String') { this.format = format; } } diff --git a/unitTests/components/mcp/tools/application.test.js b/unitTests/components/mcp/tools/application.test.js index 1ce2eb33c2..ff9d807ba6 100644 --- a/unitTests/components/mcp/tools/application.test.js +++ b/unitTests/components/mcp/tools/application.test.js @@ -1087,3 +1087,127 @@ describe('mcp/tools/application — custom mcpResources opt-in (#1609)', () => { assert.equal(matchCustomResource('application', 'docs:///index'), undefined); }); }); + +describe('mcp/tools/application — #1920 programmatic `static properties` + docstrings', () => { + beforeEach(() => { + _resetRegistryForTest(); + _setRequestTargetForTest(FakeRequestTarget); + }); + + afterEach(() => { + _resetRegistryForTest(); + _setResourcesForTest(undefined); + _setRequestTargetForTest(undefined); + _resetApplicationToolsRegisteredForTest(); + }); + + // A programmatic Resource: declares `static properties` (Record) and NO `attributes` Array. + function makeProgrammaticResource({ path, tableName, description, properties }) { + class Cls {} + Cls.databaseName = 'data'; + Cls.tableName = tableName; + Cls.primaryKey = 'id'; + if (description) Cls.description = description; + if (properties) Cls.properties = properties; + for (const v of ['get', 'put', 'patch', 'delete', 'search', 'post']) Cls.prototype[v] = function () {}; + Cls.get = async (t) => ({ id: t.id }); + Cls.put = async () => ({ ok: true }); + Cls.patch = async () => ({ ok: true }); + Cls.post = async (_t, d) => ({ created: true, ...d }); + Cls.delete = async () => ({ deleted: true }); + Cls.search = async () => []; + return { path, Resource: Cls }; + } + + it('derives a rich inputSchema from `static properties` when no attributes are declared', () => { + const Widget = makeProgrammaticResource({ + path: 'Widget', + tableName: 'widget', + properties: { + id: { type: 'string', primaryKey: true }, + label: { type: 'string', description: 'Human-readable label' }, + size: { type: 'integer', description: 'Width in pixels' }, + status: { type: 'string', enum: ['active', 'archived'] }, + tags: { type: 'array', items: { type: 'string' } }, + }, + }); + _setResourcesForTest(makeRegistry([['Widget', { Resource: Widget.Resource }]])); + registerApplicationTools(); + const create = getTool('create_Widget'); + assert.ok(create, 'create_Widget registered'); + // Non-skeletal: each declared property surfaces with its type AND description. + assert.equal(create.inputSchema.properties.label.type, 'string'); + assert.equal(create.inputSchema.properties.label.description, 'Human-readable label'); + assert.equal(create.inputSchema.properties.size.type, 'integer'); + assert.equal(create.inputSchema.properties.size.description, 'Width in pixels'); + // enum and array shapes survive too (per cross-model review). + assert.deepEqual(create.inputSchema.properties.status.enum, ['active', 'archived']); + assert.equal(create.inputSchema.properties.tags.type, 'array'); + assert.equal(create.inputSchema.properties.tags.items.type, 'string'); + }); + + it('prefixes the verb-tool description with the class docstring / static description', () => { + const Widget = makeProgrammaticResource({ + path: 'Widget', + tableName: 'widget', + description: 'A widget in the catalog.', + properties: { id: { type: 'string', primaryKey: true }, label: { type: 'string', description: 'The label' } }, + }); + _setResourcesForTest(makeRegistry([['Widget', { Resource: Widget.Resource }]])); + registerApplicationTools(); + const get = getTool('get_Widget'); + assert.ok(get, 'get_Widget registered'); + assert.ok( + get.description.includes('A widget in the catalog.'), + `expected the docstring prefix on the tool description, got: ${get.description}` + ); + }); + + it('carries per-attribute descriptions from a table-backed Resource into the tool inputSchema', () => { + // The table-backed path (real attributes carrying descriptions, as the GraphQL parser emits). + const Product = makeTableResource({ + databaseName: 'data', + tableName: 'product', + attributes: [ + { name: 'id', isPrimaryKey: true, type: 'String' }, + { name: 'sku', type: 'String', description: 'Stock keeping unit' }, + ], + }); + Product.description = 'A product record.'; + _setResourcesForTest(makeRegistry([['Product', { Resource: Product }]])); + registerApplicationTools(); + const create = getTool('create_Product'); + assert.ok(create, 'create_Product registered'); + assert.equal(create.inputSchema.properties.sku.description, 'Stock keeping unit'); + const get = getTool('get_Product'); + assert.ok(get.description.includes('A product record.'), `expected docstring prefix, got: ${get.description}`); + }); + + it('inherits `static properties` through class extension', () => { + class Base {} + Base.databaseName = 'data'; + Base.primaryKey = 'id'; + Base.properties = { + id: { type: 'string', primaryKey: true }, + label: { type: 'string', description: 'inherited label' }, + }; + class Special extends Base {} + Special.tableName = 'special'; + for (const v of ['get', 'put', 'patch', 'delete', 'search', 'post']) Special.prototype[v] = function () {}; + Special.get = async (t) => ({ id: t.id }); + Special.put = async () => ({}); + Special.patch = async () => ({}); + Special.post = async (_t, d) => d; + Special.delete = async () => ({}); + Special.search = async () => []; + _setResourcesForTest(makeRegistry([['Special', { Resource: Special }]])); + registerApplicationTools(); + const create = getTool('create_Special'); + assert.ok(create, 'create_Special registered'); + assert.equal( + create.inputSchema.properties.label.description, + 'inherited label', + 'child should derive its schema from the inherited static properties' + ); + }); +}); diff --git a/unitTests/components/mcp/tools/convergence.test.js b/unitTests/components/mcp/tools/convergence.test.js new file mode 100644 index 0000000000..be971c0bc8 --- /dev/null +++ b/unitTests/components/mcp/tools/convergence.test.js @@ -0,0 +1,77 @@ +// #1920 — a single class-level metadata source (docstring + `static properties`) must surface on +// BOTH the MCP tool descriptors and the OpenAPI document. This is the cross-surface convergence the +// #1095 acceptance called for; previously each surface was tested in isolation. +const assert = require('node:assert'); +const { + registerApplicationTools, + _setResourcesForTest, + _setRequestTargetForTest, + _resetApplicationToolsRegisteredForTest, +} = require('#src/components/mcp/tools/application'); +const { getTool, _resetRegistryForTest } = require('#src/components/mcp/toolRegistry'); +const { generateJsonApi } = require('#src/resources/openApi'); + +class FakeRequestTarget {} + +// One programmatic Resource, shared by both surfaces. +function makeResources() { + class Widget {} + Widget.databaseName = 'data'; + Widget.tableName = 'widget'; + Widget.primaryKey = 'id'; + Widget.description = 'A widget in the catalog.'; + Widget.properties = { + id: { type: 'string', primaryKey: true }, + label: { type: 'string', description: 'Human-readable label' }, + }; + for (const v of ['get', 'put', 'patch', 'delete', 'search', 'post']) Widget.prototype[v] = function () {}; + Widget.get = async (t) => ({ id: t.id }); + Widget.put = async () => ({ ok: true }); + Widget.patch = async () => ({ ok: true }); + Widget.post = async (_t, d) => ({ created: true, ...d }); + Widget.delete = async () => ({ deleted: true }); + Widget.search = async () => []; + + const resources = new Map(); + resources.set('Widget', { path: 'Widget', Resource: Widget, hasSubPaths: false, relativeURL: '' }); + resources.allTypes = new Map(); + return resources; +} + +describe('mcp/openapi — #1920 description convergence across surfaces', () => { + beforeEach(() => { + _resetRegistryForTest(); + _setRequestTargetForTest(FakeRequestTarget); + }); + afterEach(() => { + _resetRegistryForTest(); + _setResourcesForTest(undefined); + _setRequestTargetForTest(undefined); + _resetApplicationToolsRegisteredForTest(); + }); + + it('surfaces the docstring and per-property descriptions on both MCP tools and OpenAPI', () => { + const resources = makeResources(); + + // MCP side + _setResourcesForTest(resources); + registerApplicationTools(); + const get = getTool('get_Widget'); + const create = getTool('create_Widget'); + assert.ok(get && create, 'MCP verb tools registered'); + assert.ok( + get.description.includes('A widget in the catalog.'), + `MCP get tool should carry the docstring, got: ${get.description}` + ); + assert.equal(create.inputSchema.properties.label.description, 'Human-readable label'); + + // OpenAPI side — same source + const api = generateJsonApi(resources, 'https://harper.fast'); + const schema = api.components.schemas.Widget; + assert.equal(schema.description, 'A widget in the catalog.', 'OpenAPI schema should carry the docstring'); + assert.equal(schema.properties.label.description, 'Human-readable label'); + + // Convergence: the per-property description is identical on both surfaces. + assert.equal(create.inputSchema.properties.label.description, schema.properties.label.description); + }); +}); diff --git a/unitTests/components/mcp/tools/operations.test.js b/unitTests/components/mcp/tools/operations.test.js index 3cb32da403..cc76f5d696 100644 --- a/unitTests/components/mcp/tools/operations.test.js +++ b/unitTests/components/mcp/tools/operations.test.js @@ -349,6 +349,27 @@ describe('mcp/tools/operations — registration', () => { const { tools } = listTools({ user: SUPER, profile: 'operations', sessionId: 's', limit: 200 }); assert.equal(tools.length, 0); }); + + it('does not emit idempotentHint on the actual non-idempotent tools (add_user, add_role)', () => { + // add_user / add_role / create_* are NOT idempotent under MCP semantics — a second call + // returns an "already exists" error, so annotating them idempotent nudges LLMs into retry + // loops. IDEMPOTENT_OPERATIONS ships empty; verify that against the EMITTED tool annotations, + // not just the descriptions map (which would pass trivially). + envOverrides.mcp_operations_allow = ['add_user', 'add_role', 'describe_all']; + _setOperationFunctionMapForTest( + makeOpMap([ + ['add_user', null], + ['add_role', null], + ['describe_all', null], + ]) + ); + registerOperationsTools(); + for (const name of ['add_user', 'add_role', 'describe_all']) { + const tool = getTool(name); + assert.ok(tool, `${name} registered`); + assert.notEqual(tool.annotations?.idempotentHint, true, `${name} must not be annotated idempotentHint:true`); + } + }); }); describe('mcp/tools/operations — catalog coverage lint', () => { @@ -384,12 +405,10 @@ describe('mcp/tools/operations — catalog coverage lint', () => { assert.deepEqual(missing, [], `Operations on DEFAULT_ALLOW without a description: ${missing.join(', ')}`); }); - it('OPERATION_DESCRIPTIONS does not annotate non-idempotent operations as idempotent (sanity)', () => { - // add_user / create_* are NOT idempotent under MCP semantics — second call returns - // an "already exists" error. Annotating them as idempotent nudges LLMs into - // retry loops with confusing failures. Catch via description-text sniff plus - // the IDEMPOTENT_OPERATIONS set check below. - assert.ok(!('idempotentHint' in OPERATION_DESCRIPTIONS), 'descriptions are strings, not objects'); + it('OPERATION_DESCRIPTIONS entries are strings (annotations are emitted at registration, tested there)', () => { + // idempotentHint / readOnlyHint etc. are emitted onto the registered tool, not stored here. + // The emitted-annotation behavior for non-idempotent ops is asserted in the registration suite. + assert.ok(Object.values(OPERATION_DESCRIPTIONS).every((d) => typeof d === 'string')); }); it('all description entries cite the source handler in a preceding comment', () => { diff --git a/unitTests/resources/graphqlMetadata.test.js b/unitTests/resources/graphqlMetadata.test.js index 283921b98a..7bac22e404 100644 --- a/unitTests/resources/graphqlMetadata.test.js +++ b/unitTests/resources/graphqlMetadata.test.js @@ -154,4 +154,37 @@ describe('GraphQL parser — metadata capture (#1095)', () => { assert.ok(tables.ShapeCheck.properties.title); }); }); + + // #1920: a programmatic Resource may declare `static properties` (the Record form) without an + // `attributes` Array; `projectPropertiesToAttributes` rebuilds the Array so the schema-derivation + // paths (MCP, OpenAPI) can consume it. It must be the structural inverse of the forward projection. + describe('projectPropertiesToAttributes (#1920)', () => { + const { projectPropertiesToAttributes, projectAttributesToProperties } = require('#src/resources/jsonSchemaTypes'); + + it('projects each property into a named attribute carrying type + description + flags', () => { + const attrs = projectPropertiesToAttributes({ + id: { type: 'string', primaryKey: true }, + label: { type: 'string', description: 'Human-readable label' }, + size: { type: 'integer', description: 'Width in pixels', nullable: true }, + }); + const byName = Object.fromEntries(attrs.map((a) => [a.name, a])); + assert.strictEqual(byName.id.isPrimaryKey, true); + assert.strictEqual(byName.label.type, 'string'); + assert.strictEqual(byName.label.description, 'Human-readable label'); + assert.strictEqual(byName.size.type, 'integer'); + assert.strictEqual(byName.size.nullable, true); + }); + + it('round-trips with projectAttributesToProperties (properties -> attributes -> properties)', () => { + // Includes JSON-only hints (enum, format) and nested/array shapes to prove nothing is dropped. + const properties = { + sku: { type: 'string', description: 'Stock keeping unit', primaryKey: true }, + status: { type: 'string', enum: ['active', 'archived'], format: 'x-status' }, + tags: { type: 'array', items: { type: 'string' } }, + dims: { type: 'object', properties: { w: { type: 'integer' }, h: { type: 'integer' } } }, + }; + const roundTripped = projectAttributesToProperties(projectPropertiesToAttributes(properties)); + assert.deepStrictEqual(roundTripped, properties); + }); + }); }); diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index 0a44e9e7c8..32467ca86d 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -254,4 +254,58 @@ describe('test openApi module', () => { expect(api.paths).not.to.have.property('/secret/{id}'); }); }); + + describe('#1920 programmatic `static properties`', () => { + function programmaticResources() { + const r = new Map(); + r.set('Widget', { + path: 'Widget', + Resource: { + prototype: { get: () => [], put: () => [], patch: () => [], delete: () => [], post: () => [] }, + description: 'A widget in the catalog.', + // Record form only — no `attributes` Array. + properties: { + id: { type: 'string', primaryKey: true }, + label: { type: 'string', description: 'Human-readable label' }, + size: { type: 'integer', description: 'Width in pixels' }, + }, + }, + }); + r.allTypes = new Map(); + return r; + } + + it('emits per-property schemas (type + description) from a bare static-properties declaration', () => { + const api = generateJsonApi(programmaticResources(), serverURL); + const schema = api.components.schemas.Widget; + expect(schema).to.have.property('description', 'A widget in the catalog.'); + expect(schema.properties.label).to.include({ type: 'string', description: 'Human-readable label' }); + expect(schema.properties.size).to.include({ type: 'integer', description: 'Width in pixels' }); + }); + + it('emits array items, enum, and nested-object shapes (not undefined/skeletal)', () => { + const r = new Map(); + r.set('Gadget', { + path: 'Gadget', + Resource: { + prototype: { get: () => [], put: () => [] }, + properties: { + id: { type: 'string', primaryKey: true }, + tags: { type: 'array', items: { type: 'string' } }, + status: { type: 'string', enum: ['active', 'archived'] }, + dims: { type: 'object', properties: { w: { type: 'integer' }, h: { type: 'integer' } } }, + }, + }, + }); + r.allTypes = new Map(); + const schema = generateJsonApi(r, serverURL).components.schemas.Gadget; + // array-of-scalar: items carry the JSON type, not `undefined` + expect(schema.properties.tags).to.deep.include({ type: 'array', items: { type: 'string' } }); + // enum surfaces the allowed values + expect(schema.properties.status.enum).to.deep.equal(['active', 'archived']); + // nested object is recursed, not emitted as a bare/undefined object + expect(schema.properties.dims.type).to.equal('object'); + expect(schema.properties.dims.properties.w).to.include({ type: 'integer' }); + }); + }); }); From 6ac0d1490c94f7696bdd75fbcc5c4e64c1eb7212 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 16:34:11 -0600 Subject: [PATCH 02/10] fix(mcp/openapi): preserve union/nullable, nested constraints, array-of-object (deep+code review) Addresses findings from the deep-review + cross-model review of #1921: - Union type array (e.g. `type: ['string','null']`) no longer truncates to its first member; a `'null'` member folds into `nullable` and the remaining type is kept. - Nested-object `required` / `additionalProperties` now survive the properties<->attributes round-trip and are emitted on both MCP and OpenAPI. - OpenAPI array-of-object elements are recursed into their full object schema instead of emitting an undefined-typed item (MCP already handled this). - JSON-Schema scalar type tolerance is now a single shared set (`JSON_SCHEMA_SCALAR_TYPES`) consumed by both type mappers, so they can't drift. - Projected array-element attribute no longer reuses the parent field name. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/tools/schemas/derive.ts | 19 ++++++----- resources/jsonSchemaTypes.ts | 35 +++++++++++++++++++-- resources/openApi.ts | 21 +++++++------ unitTests/resources/graphqlMetadata.test.js | 16 ++++++++-- unitTests/resources/openApi.test.js | 14 +++++++-- 5 files changed, 80 insertions(+), 25 deletions(-) diff --git a/components/mcp/tools/schemas/derive.ts b/components/mcp/tools/schemas/derive.ts index 984d159f50..1f9866c482 100644 --- a/components/mcp/tools/schemas/derive.ts +++ b/components/mcp/tools/schemas/derive.ts @@ -11,6 +11,8 @@ * doesn't waste tokens on fields it can't write), NOT a security boundary. */ +import { JSON_SCHEMA_SCALAR_TYPES } from '../../../../resources/jsonSchemaTypes.ts'; + export interface HarperAttribute { name: string; type?: string; @@ -29,6 +31,8 @@ export interface HarperAttribute { enum?: readonly (string | number | boolean | null)[]; format?: string; const?: unknown; + required?: readonly string[]; + additionalProperties?: boolean; } export interface AttributePermissionEntry { @@ -46,6 +50,9 @@ type Mode = 'read' | 'insert' | 'update'; * than blocking the field entirely; the runtime will validate. */ function harperTypeToJsonSchema(type: string | undefined): { type: string | string[] } | object { + // A programmatic Resource's `static properties` already speaks JSON Schema (lowercase types, no + // collision with Harper's capitalized GraphQL types); pass those through unchanged. + if (type && JSON_SCHEMA_SCALAR_TYPES.has(type)) return { type }; switch (type) { case 'Int': case 'Long': @@ -67,16 +74,6 @@ function harperTypeToJsonSchema(type: string | undefined): { type: string | stri case 'Any': case undefined: return {}; - case 'string': - case 'integer': - case 'number': - case 'boolean': - case 'object': - case 'array': - case 'null': - // A programmatic Resource's `static properties` already speaks JSON Schema (lowercase types, - // no collision with Harper's capitalized GraphQL types); pass those through unchanged. - return { type }; default: return { type: 'string' }; } @@ -93,6 +90,8 @@ function attributeToProperty(attr: HarperAttribute): object { type: 'object', properties: Object.fromEntries(attr.properties.map((p) => [p.name, attributeToProperty(p)])), }; + if (attr.required) base.required = attr.required; + if (attr.additionalProperties !== undefined) base.additionalProperties = attr.additionalProperties; } else if (attr.type === 'array' && attr.elements) { base = { type: 'array', diff --git a/resources/jsonSchemaTypes.ts b/resources/jsonSchemaTypes.ts index 779388fa31..f5bc5c4abf 100644 --- a/resources/jsonSchemaTypes.ts +++ b/resources/jsonSchemaTypes.ts @@ -30,6 +30,21 @@ export interface JsonSchemaFragment { const?: unknown; } +/** + * The JSON-Schema scalar/structural type names. A programmatic Resource's `static properties` speaks + * JSON Schema directly (lowercase), so the MCP and OpenAPI type mappers pass these through unchanged + * rather than treating them as unknown Harper types. Shared so the two mappers can't drift apart. + */ +export const JSON_SCHEMA_SCALAR_TYPES: ReadonlySet = new Set([ + 'string', + 'integer', + 'number', + 'boolean', + 'object', + 'array', + 'null', +]); + export const DATA_TYPES: Record = { Int: 'integer', Float: 'number', @@ -64,6 +79,9 @@ export interface AttributeLike { enum?: readonly (string | number | boolean | null)[]; format?: string; const?: unknown; + /** Object-level constraints for a nested object field, carried through the round-trip. */ + required?: readonly string[]; + additionalProperties?: boolean; } /** @@ -84,6 +102,8 @@ export function attributeToFragment(attr: AttributeLike): JsonSchemaFragment { fragment.type = 'object'; fragment.properties = {}; for (const sub of attr.properties) fragment.properties[sub.name] = attributeToFragment(sub); + if (attr.required) fragment.required = attr.required; + if (attr.additionalProperties !== undefined) fragment.additionalProperties = attr.additionalProperties; } else if (attr.type === 'array' && attr.elements) { fragment.type = 'array'; fragment.items = attributeToFragment(attr.elements); @@ -128,11 +148,22 @@ function fragmentToAttribute(name: string, fragment: JsonSchemaFragment): Attrib const attr: AttributeLike = { name }; if (fragment.properties) { attr.properties = Object.entries(fragment.properties).map(([subName, sub]) => fragmentToAttribute(subName, sub)); + if (fragment.required) attr.required = fragment.required; + if (fragment.additionalProperties !== undefined) attr.additionalProperties = fragment.additionalProperties; } else if (fragment.type === 'array' && fragment.items) { attr.type = 'array'; - attr.elements = fragmentToAttribute(name, fragment.items); + // The element attribute's name is unused (attributeToFragment ignores it); keep it empty rather + // than misleadingly reusing the array field's own name. + attr.elements = fragmentToAttribute('', fragment.items); + } else if (Array.isArray(fragment.type)) { + // JSON-Schema union type. Fold a `'null'` member into `nullable` (the OpenAPI-expressible form) + // and keep the remaining member. A single non-null member is the common `['T','null']` case; a + // genuine multi-type union isn't expressible on an attribute, so the first member is kept. + const members = fragment.type.filter((t) => t !== 'null'); + if (members.length !== fragment.type.length) attr.nullable = true; + if (members.length > 0) attr.type = members[0]; } else if (fragment.type != null) { - attr.type = Array.isArray(fragment.type) ? fragment.type[0] : fragment.type; + attr.type = fragment.type; } if (fragment.description) attr.description = fragment.description; if (fragment.primaryKey) attr.isPrimaryKey = true; diff --git a/resources/openApi.ts b/resources/openApi.ts index bce5f80690..a7c9b78b26 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -1,15 +1,15 @@ import { packageJson } from '../utility/packageUtils.js'; import { Resources, routePatternToTemplate } from './Resources.ts'; import { Resource } from './Resource.ts'; -import { DATA_TYPES, attributeToFragment, projectPropertiesToAttributes } from './jsonSchemaTypes.ts'; +import { + DATA_TYPES, + JSON_SCHEMA_SCALAR_TYPES, + attributeToFragment, + projectPropertiesToAttributes, +} from './jsonSchemaTypes.ts'; const OPENAPI_VERSION = '3.0.3'; -// A programmatic Resource's `static properties` uses JSON Schema types directly (lowercase). Harper's -// GraphQL attribute types are all capitalized and live in DATA_TYPES, so a lowercase scalar reaching -// the attribute mapper came from `static properties` and should be emitted as-is. -const JSON_SCHEMA_SCALARS = new Set(['string', 'integer', 'number', 'boolean', 'object', 'array', 'null']); - const SCHEMA_COMP_REF = '#/components/schemas/'; const DESCRIPTION_200 = 'successful operation'; @@ -166,16 +166,19 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { // schema (sub-properties recursed); OpenAPI's table path uses $refs instead. props[name] = attributeToFragment(attr); } else if (type === 'array') { - if (elements.type === 'Any') { + if (elements.properties) { + // array of nested objects — project the element to its full object schema + props[name] = { type: 'array', items: attributeToFragment(elements) }; + } else if (elements.type === 'Any') { props[name] = { type: 'array', items: { format: elements.type } }; - } else if (!DATA_TYPES[elements.type] && JSON_SCHEMA_SCALARS.has(elements.type)) { + } else if (!DATA_TYPES[elements.type] && JSON_SCHEMA_SCALAR_TYPES.has(elements.type)) { props[name] = { type: 'array', items: new Type(elements.type) }; } else { props[name] = { type: 'array', items: new Type(DATA_TYPES[elements.type], elements.type) }; } } else if (type === 'Any') { props[name] = { format: type }; - } else if (!DATA_TYPES[type] && JSON_SCHEMA_SCALARS.has(type)) { + } else if (!DATA_TYPES[type] && JSON_SCHEMA_SCALAR_TYPES.has(type)) { props[name] = new Type(type); } else { props[name] = new Type(DATA_TYPES[type], type); diff --git a/unitTests/resources/graphqlMetadata.test.js b/unitTests/resources/graphqlMetadata.test.js index 7bac22e404..b6ce1c3e6e 100644 --- a/unitTests/resources/graphqlMetadata.test.js +++ b/unitTests/resources/graphqlMetadata.test.js @@ -176,15 +176,27 @@ describe('GraphQL parser — metadata capture (#1095)', () => { }); it('round-trips with projectAttributesToProperties (properties -> attributes -> properties)', () => { - // Includes JSON-only hints (enum, format) and nested/array shapes to prove nothing is dropped. + // Includes JSON-only hints (enum, format), nested/array shapes, and nested object-level + // constraints (required/additionalProperties) to prove nothing is dropped. const properties = { sku: { type: 'string', description: 'Stock keeping unit', primaryKey: true }, status: { type: 'string', enum: ['active', 'archived'], format: 'x-status' }, tags: { type: 'array', items: { type: 'string' } }, - dims: { type: 'object', properties: { w: { type: 'integer' }, h: { type: 'integer' } } }, + dims: { + type: 'object', + required: ['w'], + additionalProperties: false, + properties: { w: { type: 'integer' }, h: { type: 'integer' } }, + }, }; const roundTripped = projectAttributesToProperties(projectPropertiesToAttributes(properties)); assert.deepStrictEqual(roundTripped, properties); }); + + it('folds a JSON-Schema union `["T","null"]` into nullable (not truncated to the first member)', () => { + const [note] = projectPropertiesToAttributes({ note: { type: ['string', 'null'] } }); + assert.strictEqual(note.type, 'string'); + assert.strictEqual(note.nullable, true, 'the null member must become nullable, not be dropped'); + }); }); }); diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index 32467ca86d..f2d415202b 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -293,7 +293,12 @@ describe('test openApi module', () => { id: { type: 'string', primaryKey: true }, tags: { type: 'array', items: { type: 'string' } }, status: { type: 'string', enum: ['active', 'archived'] }, - dims: { type: 'object', properties: { w: { type: 'integer' }, h: { type: 'integer' } } }, + dims: { + type: 'object', + required: ['w'], + properties: { w: { type: 'integer' }, h: { type: 'integer' } }, + }, + rows: { type: 'array', items: { type: 'object', properties: { x: { type: 'integer' } } } }, }, }, }); @@ -303,9 +308,14 @@ describe('test openApi module', () => { expect(schema.properties.tags).to.deep.include({ type: 'array', items: { type: 'string' } }); // enum surfaces the allowed values expect(schema.properties.status.enum).to.deep.equal(['active', 'archived']); - // nested object is recursed, not emitted as a bare/undefined object + // nested object is recursed, with its object-level constraints preserved expect(schema.properties.dims.type).to.equal('object'); expect(schema.properties.dims.properties.w).to.include({ type: 'integer' }); + expect(schema.properties.dims.required).to.deep.equal(['w']); + // array-of-object: items are the recursed object shape, not an undefined-typed blob + expect(schema.properties.rows.type).to.equal('array'); + expect(schema.properties.rows.items.type).to.equal('object'); + expect(schema.properties.rows.items.properties.x).to.include({ type: 'integer' }); }); }); }); From 6539146e42652ad68fbd6e92c6112deb8be3ef45 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 16:37:37 -0600 Subject: [PATCH 03/10] test(mcp): cover const, nested-object constraints, and array-of-object on the MCP inputSchema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symmetry with the OpenAPI-side assertions — these behaviors were enabled on both surfaces but only asserted on OpenAPI. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- .../components/mcp/tools/application.test.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/unitTests/components/mcp/tools/application.test.js b/unitTests/components/mcp/tools/application.test.js index ff9d807ba6..bd948d35d0 100644 --- a/unitTests/components/mcp/tools/application.test.js +++ b/unitTests/components/mcp/tools/application.test.js @@ -1146,6 +1146,35 @@ describe('mcp/tools/application — #1920 programmatic `static properties` + doc assert.equal(create.inputSchema.properties.tags.items.type, 'string'); }); + it('carries const, nested-object required, and array-of-object into the MCP inputSchema', () => { + const Widget = makeProgrammaticResource({ + path: 'Widget', + tableName: 'widget', + properties: { + id: { type: 'string', primaryKey: true }, + kind: { type: 'string', const: 'widget' }, + dims: { + type: 'object', + required: ['w'], + additionalProperties: false, + properties: { w: { type: 'integer' }, h: { type: 'integer' } }, + }, + rows: { type: 'array', items: { type: 'object', properties: { x: { type: 'integer' } } } }, + }, + }); + _setResourcesForTest(makeRegistry([['Widget', { Resource: Widget.Resource }]])); + registerApplicationTools(); + const create = getTool('create_Widget'); + assert.ok(create, 'create_Widget registered'); + assert.equal(create.inputSchema.properties.kind.const, 'widget'); + assert.deepEqual(create.inputSchema.properties.dims.required, ['w']); + assert.equal(create.inputSchema.properties.dims.additionalProperties, false); + assert.equal(create.inputSchema.properties.dims.properties.w.type, 'integer'); + assert.equal(create.inputSchema.properties.rows.type, 'array'); + assert.equal(create.inputSchema.properties.rows.items.type, 'object'); + assert.equal(create.inputSchema.properties.rows.items.properties.x.type, 'integer'); + }); + it('prefixes the verb-tool description with the class docstring / static description', () => { const Widget = makeProgrammaticResource({ path: 'Widget', From b895a63214514455ca34d34c91d2918b719efabb Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 17:04:42 -0600 Subject: [PATCH 04/10] fix(schema): keep enum/format/const out of the front-end-neutral properties projection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught a real invariant break: emitting enum/format/const from the shared `attributeToFragment` made a code-first `types.enum` column's `Table.properties` diverge from its GraphQL `String` equivalent, breaking the deliberate code-first ⇔ GraphQL parity (types.enum is advisory — defineTable.ts). Those hints still surface on the client-facing MCP/OpenAPI schemas for programmatic Resources via derive.ts / openApi.ts; the canonical `.properties` Record stays neutral. Round-trip test updated: `.properties` projection preserves type/description/ nested shapes/required/additionalProperties; enum/format/const are surfaced by the schema paths, not carried through the neutral projection. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- resources/jsonSchemaTypes.ts | 7 ++++--- unitTests/resources/graphqlMetadata.test.js | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/resources/jsonSchemaTypes.ts b/resources/jsonSchemaTypes.ts index f5bc5c4abf..1a4b4051fb 100644 --- a/resources/jsonSchemaTypes.ts +++ b/resources/jsonSchemaTypes.ts @@ -118,9 +118,10 @@ export function attributeToFragment(attr: AttributeLike): JsonSchemaFragment { if (attr.assignUpdatedTime) fragment.assignUpdatedTime = true; if (attr.hidden) fragment.hidden = true; if (attr.nullable) fragment.nullable = true; - if (attr.enum) fragment.enum = attr.enum; - if (attr.format) fragment.format = attr.format; - if (attr.const !== undefined) fragment.const = attr.const; + // NOTE: enum/format/const are deliberately NOT emitted here. This projector feeds the canonical, + // front-end-neutral `Table.properties` Record, where a code-first `types.enum` column must stay + // identical to its GraphQL `String` equivalent (types.enum is advisory — see defineTable.ts). The + // MCP/OpenAPI schema paths (derive.ts / openApi.ts) surface those hints for programmatic Resources. return fragment; } diff --git a/unitTests/resources/graphqlMetadata.test.js b/unitTests/resources/graphqlMetadata.test.js index b6ce1c3e6e..5cec03f946 100644 --- a/unitTests/resources/graphqlMetadata.test.js +++ b/unitTests/resources/graphqlMetadata.test.js @@ -176,11 +176,12 @@ describe('GraphQL parser — metadata capture (#1095)', () => { }); it('round-trips with projectAttributesToProperties (properties -> attributes -> properties)', () => { - // Includes JSON-only hints (enum, format), nested/array shapes, and nested object-level - // constraints (required/additionalProperties) to prove nothing is dropped. + // The `.properties` projection is front-end-neutral: type, description, primaryKey, + // nested/array shapes, and nested object-level constraints (required/additionalProperties) + // survive. enum/format/const deliberately do NOT (they'd break code-first ⇔ GraphQL parity + // for `types.enum`; the MCP/OpenAPI schema paths surface those instead — see below). const properties = { sku: { type: 'string', description: 'Stock keeping unit', primaryKey: true }, - status: { type: 'string', enum: ['active', 'archived'], format: 'x-status' }, tags: { type: 'array', items: { type: 'string' } }, dims: { type: 'object', From 1383b6e423216d3d754a44e171cfe475124f6bb2 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Thu, 23 Jul 2026 17:28:08 -0600 Subject: [PATCH 05/10] fix(openapi): guard `type: 'array'` with no `items` (was a crash) A programmatic `static properties` field of `{ type: 'array' }` with no `items` (valid JSON Schema) projected to an attribute with undefined `elements`; the OpenAPI array branch then dereferenced `elements.properties`, throwing and crashing generation of the whole document. Emit a bare `{ type: 'array' }`. Flagged by gemini-code-assist + claude PR review. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- resources/openApi.ts | 5 ++++- unitTests/resources/openApi.test.js | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/resources/openApi.ts b/resources/openApi.ts index a7c9b78b26..3c0b7e081a 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -166,7 +166,10 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { // schema (sub-properties recursed); OpenAPI's table path uses $refs instead. props[name] = attributeToFragment(attr); } else if (type === 'array') { - if (elements.properties) { + if (!elements) { + // `{ type: 'array' }` with no items — valid JSON Schema (array of anything). + props[name] = { type: 'array' }; + } else if (elements.properties) { // array of nested objects — project the element to its full object schema props[name] = { type: 'array', items: attributeToFragment(elements) }; } else if (elements.type === 'Any') { diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index f2d415202b..5216ad0895 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -299,6 +299,7 @@ describe('test openApi module', () => { properties: { w: { type: 'integer' }, h: { type: 'integer' } }, }, rows: { type: 'array', items: { type: 'object', properties: { x: { type: 'integer' } } } }, + anything: { type: 'array' }, // no items — valid JSON Schema, must not crash generation }, }, }); @@ -316,6 +317,8 @@ describe('test openApi module', () => { expect(schema.properties.rows.type).to.equal('array'); expect(schema.properties.rows.items.type).to.equal('object'); expect(schema.properties.rows.items.properties.x).to.include({ type: 'integer' }); + // array with no items: emitted as a bare array (no crash on undefined elements) + expect(schema.properties.anything).to.deep.equal({ type: 'array' }); }); }); }); From d10658161f902fb3aa34ab031defbf8a639ef394 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 27 Jul 2026 10:26:39 -0600 Subject: [PATCH 06/10] fix(openapi): emit only keywords the declared 3.0.3 dialect defines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The document declares OpenAPI 3.0.3, whose Schema Object is the JSON Schema draft-04 subset. `const` arrived in draft-06 and `'null'` is not a 3.0 type, so both are keywords the declared dialect does not define — strict validators reject the result. - `const` is emitted as the equivalent single-value `enum`. - A `type: 'null'` property becomes an untyped `{ nullable: true }`, 3.0's only expression of nullability. Adds a dialect-compliance test that walks the entire generated document rather than checking a single property, so any future emit path reaching for a newer keyword fails here instead of in a consumer's tooling. Raised by kriszyp in review of #1921. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- resources/openApi.ts | 14 ++++-- unitTests/resources/openApi.test.js | 66 +++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/resources/openApi.ts b/resources/openApi.ts index 3c0b7e081a..9d4ac41481 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -182,19 +182,25 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { } else if (type === 'Any') { props[name] = { format: type }; } else if (!DATA_TYPES[type] && JSON_SCHEMA_SCALAR_TYPES.has(type)) { - props[name] = new Type(type); + // OpenAPI 3.0.3 has no `'null'` type — nullability is expressed by the `nullable` + // keyword, so a bare `type: 'null'` becomes an untyped nullable schema. + props[name] = type === 'null' ? { nullable: true } : new Type(type); } else { props[name] = new Type(DATA_TYPES[type], type); } } - // Attach per-property JSON-Schema hints (description/enum/format/const) so they surface in + // Attach per-property JSON-Schema hints (description/enum/format) so they surface in // Swagger UI / Redoc; enum in particular tells clients the allowed values. + // + // This document declares OpenAPI 3.0.3, whose Schema Object is the JSON Schema draft-04 + // subset: `const` only arrived in draft-06, so it is not a keyword here. Emit the + // equivalent single-value `enum` instead of a keyword the declared dialect doesn't define. if (props[name] && typeof props[name] === 'object' && !('$ref' in props[name])) { - const prop = props[name] as { description?: string; enum?: unknown; format?: string; const?: unknown }; + const prop = props[name] as { description?: string; enum?: unknown; format?: string }; if (description) prop.description = description; if (attr.enum && prop.enum === undefined) prop.enum = attr.enum; if (attr.format && prop.format === undefined) prop.format = attr.format; - if (attr.const !== undefined && prop.const === undefined) prop.const = attr.const; + if (attr.const !== undefined && prop.enum === undefined) prop.enum = [attr.const]; } queryParamsArray.push(new Parameter(name, 'query', props[name])); } diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index 5216ad0895..76d2423a6e 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -322,3 +322,69 @@ describe('test openApi module', () => { }); }); }); + +// The document declares OpenAPI 3.0.3, whose Schema Object is the JSON Schema draft-04 subset. +// Keywords from later drafts (`const`, added in draft-06) and JSON Schema's `'null'` type are not +// part of that dialect, so emitting them produces a document strict validators reject. This walks +// the whole generated document rather than checking one property, so any future emit path that +// reaches for a newer keyword is caught here instead of in a consumer's tooling. +describe('openApi — declared dialect compliance (3.0.3)', () => { + // Keywords absent from the draft-04 subset OpenAPI 3.0.x is built on. + const POST_DRAFT4_KEYWORDS = ['const', 'contentEncoding', 'contentMediaType', 'if', 'then', 'else', '$defs']; + + function walk(node, visit, path = '$') { + if (node === null || typeof node !== 'object') return; + if (Array.isArray(node)) { + node.forEach((item, i) => walk(item, visit, `${path}[${i}]`)); + return; + } + visit(node, path); + for (const [key, value] of Object.entries(node)) walk(value, visit, `${path}.${key}`); + } + + function buildDocument() { + class Widget {} + Widget.primaryKey = 'id'; + Widget.properties = { + id: { type: 'string', primaryKey: true }, + kind: { type: 'string', const: 'widget' }, + nothing: { type: 'null' }, + maybe: { type: ['string', 'null'] }, + nested: { type: 'object', properties: { inner: { type: 'string', const: 'x' } } }, + list: { type: 'array', items: { type: 'string', const: 'y' } }, + }; + Widget.prototype.get = function () {}; + const resources = new Map(); + resources.set('Widget', { path: 'Widget', Resource: Widget, hasSubPaths: false, relativeURL: '' }); + resources.allTypes = new Map(); + return generateJsonApi(resources, 'https://harper.fast'); + } + + it('declares 3.0.x', () => { + expect(buildDocument().openapi).to.match(/^3\.0\./); + }); + + it('emits no post-draft-04 keywords anywhere in the document', () => { + const offenders = []; + walk(buildDocument(), (node, path) => { + for (const keyword of POST_DRAFT4_KEYWORDS) { + if (Object.hasOwn(node, keyword)) offenders.push(`${path}.${keyword}`); + } + }); + expect(offenders, `post-draft-04 keywords in a 3.0.3 document: ${offenders.join(', ')}`).to.deep.equal([]); + }); + + it('never emits `type: "null"`, which 3.0.x does not define', () => { + const offenders = []; + walk(buildDocument(), (node, path) => { + const t = node.type; + if (t === 'null' || (Array.isArray(t) && t.includes('null'))) offenders.push(`${path}.type`); + }); + expect(offenders, `\`null\` types in a 3.0.3 document: ${offenders.join(', ')}`).to.deep.equal([]); + }); + + it('translates `const` to a single-value `enum`', () => { + const schema = buildDocument().components.schemas.Widget; + expect(schema.properties.kind.enum).to.deep.equal(['widget']); + }); +}); From 1eb8aa25b5515ee8e19056ea4cbec7aedcb48bb1 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 27 Jul 2026 11:00:57 -0600 Subject: [PATCH 07/10] fix(openapi): make the 3.0.3 translation hold on the nested and array paths too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit patched only the top-level scalar branch, so this PR standalone still emitted `type: "null"` inside nested objects and array items, and leaked Harper's `hidden`/`primaryKey` directives as schema keywords through the canonical projection. Its dialect test checked only for `type: "null"` and so passed on all of it — `type: 'Text'` and `type: 'Any'` sailed through too. `attributeToFragment` can't be bent to fix this: its output is also `Table.properties`, which must stay front-end-neutral. Translate on the way out instead — `toOpenApiDialect` walks an emitted fragment recursively, dropping Harper directives, resolving `'null'`/type-unions to `nullable`, and converting `const` to a single-value `enum`. Also: an author-declared `format` now outranks the Harper type name, so `{ type: 'Date', format: 'date-time' }` stops emitting `format: Date`. The test now asserts the closed six-value type enum, forbids type arrays and Harper directives, walks the serialized document (so it judges what a consumer receives rather than undefined-valued keys that vanish on the wire), and asserts the fixture's properties exist so the walks can't pass vacuously. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- resources/openApi.ts | 64 +++++++++++++++++++++++++++-- unitTests/resources/openApi.test.js | 54 ++++++++++++++++++++---- 2 files changed, 106 insertions(+), 12 deletions(-) diff --git a/resources/openApi.ts b/resources/openApi.ts index 9d4ac41481..4edb4fe56c 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -6,10 +6,62 @@ import { JSON_SCHEMA_SCALAR_TYPES, attributeToFragment, projectPropertiesToAttributes, + type JsonSchemaFragment, } from './jsonSchemaTypes.ts'; const OPENAPI_VERSION = '3.0.3'; +/** + * Convert a canonical `Table.properties` fragment into one this document's declared dialect accepts. + * + * `attributeToFragment` produces the front-end-neutral projection: it speaks current JSON Schema and + * carries Harper's own directives, neither of which belongs in an emitted 3.0.3 document. Rather than + * bend that projector (its output is also `Table.properties`, which must stay neutral), translate on + * the way out — recursively, since nested objects and array items are the paths that leak. + */ +function toOpenApiDialect(fragment: JsonSchemaFragment): JsonSchemaFragment { + // `hidden` / `primaryKey` / the timestamp flags are Harper behavior, not schema vocabulary. + const { + hidden: _hidden, + primaryKey: _primaryKey, + assignCreatedTime: _assignCreatedTime, + assignUpdatedTime: _assignUpdatedTime, + const: constValue, + ...rest + } = fragment; + const out: JsonSchemaFragment = rest; + // 3.0 has no `'null'` type and no type unions; nullability is the `nullable` keyword alone. + if (Array.isArray(out.type)) { + const members = out.type.filter((t) => t !== 'null'); + if (members.length !== out.type.length) out.nullable = true; + if (members.length > 0) out.type = members[0]; + else delete out.type; + } else if (out.type === 'null') { + delete out.type; + } + // `const` is draft-06; emit the equivalent single-value `enum`, intersecting when both are declared. + if (constValue !== undefined) { + out.enum = Array.isArray(out.enum) ? out.enum.filter((v) => v === constValue) : [constValue as never]; + } + // 3.0's `nullable` does not widen an `enum` — without `null` in the list a validator rejects it. + if (out.nullable && Array.isArray(out.enum) && !out.enum.includes(null)) out.enum = [...out.enum, null]; + if (out.properties) { + const translated: Record = {}; + for (const [key, sub] of Object.entries(out.properties)) { + if (sub.hidden) continue; // a hidden sub-property must not surface, and `required` follows below + translated[key] = toOpenApiDialect(sub); + } + out.properties = translated; + if (out.required) { + const visible = out.required.filter((key) => Object.hasOwn(translated, key)); + if (visible.length > 0) out.required = visible; + else delete out.required; + } + } + if (out.items) out.items = toOpenApiDialect(out.items); + return out; +} + const SCHEMA_COMP_REF = '#/components/schemas/'; const DESCRIPTION_200 = 'successful operation'; @@ -164,18 +216,21 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { } else if (attr.properties) { // Nested object from `static properties` — the shared projector emits the full object // schema (sub-properties recursed); OpenAPI's table path uses $refs instead. - props[name] = attributeToFragment(attr); + props[name] = toOpenApiDialect(attributeToFragment(attr)); } else if (type === 'array') { if (!elements) { // `{ type: 'array' }` with no items — valid JSON Schema (array of anything). props[name] = { type: 'array' }; } else if (elements.properties) { // array of nested objects — project the element to its full object schema - props[name] = { type: 'array', items: attributeToFragment(elements) }; + props[name] = { type: 'array', items: toOpenApiDialect(attributeToFragment(elements)) }; } else if (elements.type === 'Any') { props[name] = { type: 'array', items: { format: elements.type } }; } else if (!DATA_TYPES[elements.type] && JSON_SCHEMA_SCALAR_TYPES.has(elements.type)) { - props[name] = { type: 'array', items: new Type(elements.type) }; + props[name] = + elements.type === 'null' + ? { type: 'array', items: {} } + : { type: 'array', items: new Type(elements.type) }; } else { props[name] = { type: 'array', items: new Type(DATA_TYPES[elements.type], elements.type) }; } @@ -199,7 +254,8 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { const prop = props[name] as { description?: string; enum?: unknown; format?: string }; if (description) prop.description = description; if (attr.enum && prop.enum === undefined) prop.enum = attr.enum; - if (attr.format && prop.format === undefined) prop.format = attr.format; + // An author-declared `format` outranks the Harper type name `Type()` stamps on. + if (attr.format) prop.format = attr.format; if (attr.const !== undefined && prop.enum === undefined) prop.enum = [attr.const]; } queryParamsArray.push(new Parameter(name, 'query', props[name])); diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index 76d2423a6e..975284dd9e 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -332,7 +332,12 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { // Keywords absent from the draft-04 subset OpenAPI 3.0.x is built on. const POST_DRAFT4_KEYWORDS = ['const', 'contentEncoding', 'contentMediaType', 'if', 'then', 'else', '$defs']; + // `components.securitySchemes` holds Security Scheme Objects, not Schema Objects — their `type` + // ("http", "apiKey", …) is a different vocabulary and must not be judged against Schema Object rules. + const NON_SCHEMA_SUBTREES = ['$.components.securitySchemes']; + function walk(node, visit, path = '$') { + if (NON_SCHEMA_SUBTREES.some((prefix) => path.startsWith(prefix))) return; if (node === null || typeof node !== 'object') return; if (Array.isArray(node)) { node.forEach((item, i) => walk(item, visit, `${path}[${i}]`)); @@ -350,14 +355,22 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { kind: { type: 'string', const: 'widget' }, nothing: { type: 'null' }, maybe: { type: ['string', 'null'] }, - nested: { type: 'object', properties: { inner: { type: 'string', const: 'x' } } }, + nested: { type: 'object', properties: { inner: { type: 'string', const: 'x' }, deepNull: { type: 'null' } } }, list: { type: 'array', items: { type: 'string', const: 'y' } }, + nullableEnum: { type: 'string', enum: ['a', 'b'], nullable: true }, + nullableConst: { type: 'string', const: 'fixed', nullable: true }, + bogus: { type: 'Text' }, + secret: { type: 'string', hidden: true }, + when: { type: 'Date', format: 'date-time' }, + bytes: { type: 'Bytes' }, }; Widget.prototype.get = function () {}; const resources = new Map(); resources.set('Widget', { path: 'Widget', Resource: Widget, hasSubPaths: false, relativeURL: '' }); resources.allTypes = new Map(); - return generateJsonApi(resources, 'https://harper.fast'); + // Judge what a consumer actually receives: `JSON.stringify` drops undefined-valued keys, + // so walking the live object would flag artifacts that never reach the wire. + return JSON.parse(JSON.stringify(generateJsonApi(resources, 'https://harper.fast'))); } it('declares 3.0.x', () => { @@ -374,17 +387,42 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { expect(offenders, `post-draft-04 keywords in a 3.0.3 document: ${offenders.join(', ')}`).to.deep.equal([]); }); - it('never emits `type: "null"`, which 3.0.x does not define', () => { + it('emits only the six type values 3.0.x defines, and never a type array', () => { + // 3.0's `type` is a closed enum of single values: no `'null'`, no unions, and nothing from + // Harper's own vocabulary. Checking only for `'null'` would sail past `type: 'Text'` or + // `['string','number']`, both equally invalid here. + const OPENAPI_30_TYPES = ['string', 'number', 'integer', 'boolean', 'object', 'array']; const offenders = []; walk(buildDocument(), (node, path) => { + if (!Object.hasOwn(node, 'type')) return; const t = node.type; - if (t === 'null' || (Array.isArray(t) && t.includes('null'))) offenders.push(`${path}.type`); + if (Array.isArray(t)) offenders.push(`${path}.type=[${t.join(',')}] (unions invalid in 3.0)`); + else if (!OPENAPI_30_TYPES.includes(t)) offenders.push(`${path}.type=${String(t)}`); }); - expect(offenders, `\`null\` types in a 3.0.3 document: ${offenders.join(', ')}`).to.deep.equal([]); + expect(offenders, `invalid 3.0 types: ${offenders.join(', ')}`).to.deep.equal([]); }); - it('translates `const` to a single-value `enum`', () => { - const schema = buildDocument().components.schemas.Widget; - expect(schema.properties.kind.enum).to.deep.equal(['widget']); + it('never leaks a Harper directive into the emitted document', () => { + // `hidden`/`primaryKey`/the timestamp flags drive Harper behavior; they are not schema vocabulary + // and a consumer parsing this document has no meaning for them. + const DIRECTIVES = ['hidden', 'primaryKey', 'assignCreatedTime', 'assignUpdatedTime']; + const offenders = []; + walk(buildDocument(), (node, path) => { + for (const key of DIRECTIVES) if (Object.hasOwn(node, key)) offenders.push(`${path}.${key}`); + }); + expect(offenders, `Harper directives in the document: ${offenders.join(', ')}`).to.deep.equal([]); + }); + + it('translates a top-level `const` to a single-value `enum`', () => { + const props = buildDocument().components.schemas.Widget.properties; + expect(props.kind.enum).to.deep.equal(['widget']); + expect(props.kind).to.not.have.property('const'); + }); + + it('emits the properties under test (guards the walk assertions against an empty document)', () => { + const props = buildDocument().components.schemas.Widget.properties; + for (const key of ['kind', 'nothing', 'maybe', 'nested', 'list', 'nullableEnum', 'nullableConst', 'when']) { + expect(props, `fixture property ${key} missing — walk assertions would pass vacuously`).to.have.property(key); + } }); }); From f34cedef99e2e92d95772b224eded14729be3b1f Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 27 Jul 2026 11:29:41 -0600 Subject: [PATCH 08/10] fix(openapi): carry nullability onto the top-level scalar schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scalar branch emitted the type alone, so a `static properties` field declared `{ type: 'string', nullable: true }` (or `['string','null']`, which folds to the same attribute) produced a document asserting the field rejects null — while MCP kept it nullable. The nested/array paths already translated this through toOpenApiDialect; only the top-level path did not. Also intersects `const` with a co-declared `enum` rather than deferring to the enum, matching the nested path. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- resources/openApi.ts | 19 +++++++++++++++++-- unitTests/resources/openApi.test.js | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/resources/openApi.ts b/resources/openApi.ts index 4edb4fe56c..15d1cd16ed 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -251,12 +251,27 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { // subset: `const` only arrived in draft-06, so it is not a keyword here. Emit the // equivalent single-value `enum` instead of a keyword the declared dialect doesn't define. if (props[name] && typeof props[name] === 'object' && !('$ref' in props[name])) { - const prop = props[name] as { description?: string; enum?: unknown; format?: string }; + const prop = props[name] as { + description?: string; + enum?: unknown[]; + format?: string; + nullable?: boolean; + }; if (description) prop.description = description; if (attr.enum && prop.enum === undefined) prop.enum = attr.enum; // An author-declared `format` outranks the Harper type name `Type()` stamps on. if (attr.format) prop.format = attr.format; - if (attr.const !== undefined && prop.enum === undefined) prop.enum = [attr.const]; + // Intersect rather than defer: `const` narrows an `enum` declared alongside it. + if (attr.const !== undefined) { + prop.enum = Array.isArray(prop.enum) ? prop.enum.filter((value) => value === attr.const) : [attr.const]; + } + // `type: ['string', 'null']` folds to `type: 'string'` + `nullable` upstream; without this the + // scalar path emits the type alone and the document claims the field rejects null. + if (nullable) prop.nullable = true; + // 3.0's `nullable` does not widen an `enum` — without `null` in the list a validator rejects it. + if (prop.nullable && Array.isArray(prop.enum) && !prop.enum.includes(null)) { + prop.enum = [...prop.enum, null]; + } } queryParamsArray.push(new Parameter(name, 'query', props[name])); } diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index 975284dd9e..aa8e821817 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -419,6 +419,22 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { expect(props.kind).to.not.have.property('const'); }); + it('carries nullability onto the emitted scalar schema', () => { + // The walk assertions above only prove `type: 'null'` and unions are gone; they would pass just as + // happily if nullability were dropped instead of translated. + const props = buildDocument().components.schemas.Widget.properties; + expect(props.maybe).to.deep.equal({ type: 'string', nullable: true }); + expect(props.nothing.nullable).to.equal(true); + }); + + it('widens a nullable `enum` with `null` (3.0 `nullable` does not do it)', () => { + const props = buildDocument().components.schemas.Widget.properties; + expect(props.nullableEnum.nullable).to.equal(true); + expect(props.nullableEnum.enum).to.deep.equal(['a', 'b', null]); + // `const` + `nullable`: the single-value enum still has to admit null. + expect(props.nullableConst.enum).to.deep.equal(['fixed', null]); + }); + it('emits the properties under test (guards the walk assertions against an empty document)', () => { const props = buildDocument().components.schemas.Widget.properties; for (const key of ['kind', 'nothing', 'maybe', 'nested', 'list', 'nullableEnum', 'nullableConst', 'when']) { From 2039e4deca86c2c28698b4bc94ee8b1eafa4aa93 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 27 Jul 2026 11:56:02 -0600 Subject: [PATCH 09/10] feat(schemas): preserve a declared type union instead of keeping its first member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `static properties` speaks JSON Schema, which has type unions, but the attribute form it projects into has a single `type` — so `['string','number']` silently became string-only and `['null']` became untyped. Attributes now carry the source union on `types` (`type` still holds the first non-null member for single-type consumers). Each surface then expresses it in its own dialect: MCP passes the array through, OpenAPI 3.0 emits `oneOf`, since 3.0 has neither type arrays nor a `null` type. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- components/mcp/tools/schemas/derive.ts | 6 ++++ resources/jsonSchemaTypes.ts | 20 ++++++++++-- resources/openApi.ts | 31 +++++++++++++++++-- .../components/mcp/tools/convergence.test.js | 17 ++++++++++ unitTests/resources/graphqlMetadata.test.js | 19 ++++++++++++ unitTests/resources/openApi.test.js | 29 ++++++++++++++++- 6 files changed, 116 insertions(+), 6 deletions(-) diff --git a/components/mcp/tools/schemas/derive.ts b/components/mcp/tools/schemas/derive.ts index 1f9866c482..512799574e 100644 --- a/components/mcp/tools/schemas/derive.ts +++ b/components/mcp/tools/schemas/derive.ts @@ -19,6 +19,8 @@ export interface HarperAttribute { description?: string; hidden?: boolean; nullable?: boolean; + /** Source JSON-Schema type union from `static properties`; MCP accepts type arrays, so it passes through. */ + types?: readonly string[]; isPrimaryKey?: boolean; properties?: HarperAttribute[]; elements?: HarperAttribute; @@ -97,6 +99,10 @@ function attributeToProperty(attr: HarperAttribute): object { type: 'array', items: attributeToProperty(attr.elements), }; + } else if (attr.types) { + // MCP speaks JSON Schema, which has type unions — emit the author's union as declared rather + // than the single `type` the attribute form collapses to. + base = { type: [...attr.types] }; } else { base = harperTypeToJsonSchema(attr.type) as typeof base; } diff --git a/resources/jsonSchemaTypes.ts b/resources/jsonSchemaTypes.ts index 1a4b4051fb..5194c6469b 100644 --- a/resources/jsonSchemaTypes.ts +++ b/resources/jsonSchemaTypes.ts @@ -28,6 +28,8 @@ export interface JsonSchemaFragment { additionalProperties?: boolean; format?: string; const?: unknown; + /** Emitted by the OpenAPI 3.0 projection for a genuine multi-type union; not authored directly. */ + oneOf?: JsonSchemaFragment[]; } /** @@ -71,6 +73,12 @@ export interface AttributeLike { assignCreatedTime?: boolean; assignUpdatedTime?: boolean; nullable?: boolean; + /** + * The source JSON-Schema type union, verbatim, when `static properties` declared one. `type` holds + * the first non-null member so single-type consumers keep working; surfaces that can express a + * union (MCP passes it through, OpenAPI 3.0 translates it to `oneOf`) read this instead. + */ + types?: readonly string[]; elements?: AttributeLike; /** Sub-attributes of a nested object field (the same array form `Table.validate` iterates). */ properties?: AttributeLike[]; @@ -107,6 +115,10 @@ export function attributeToFragment(attr: AttributeLike): JsonSchemaFragment { } else if (attr.type === 'array' && attr.elements) { fragment.type = 'array'; fragment.items = attributeToFragment(attr.elements); + } else if (attr.types) { + // A declared union round-trips verbatim; collapsing it to `attr.type` here would make the + // canonical `Table.properties` disagree with what the author wrote. + fragment.type = [...attr.types] as JsonSchemaType[]; } else { const jsonType = attr.type ? DATA_TYPES[attr.type] : undefined; if (jsonType) fragment.type = jsonType; @@ -157,9 +169,11 @@ function fragmentToAttribute(name: string, fragment: JsonSchemaFragment): Attrib // than misleadingly reusing the array field's own name. attr.elements = fragmentToAttribute('', fragment.items); } else if (Array.isArray(fragment.type)) { - // JSON-Schema union type. Fold a `'null'` member into `nullable` (the OpenAPI-expressible form) - // and keep the remaining member. A single non-null member is the common `['T','null']` case; a - // genuine multi-type union isn't expressible on an attribute, so the first member is kept. + // JSON-Schema union type. Keep the source union on `types` so surfaces that can express one + // (MCP natively, OpenAPI 3.0 via `oneOf`) don't have to reconstruct it, and fold a `'null'` + // member into `nullable` as well since that is the form OpenAPI needs. `type` carries the first + // non-null member for the single-type consumers (validation, query coercion) that read it. + attr.types = fragment.type; const members = fragment.type.filter((t) => t !== 'null'); if (members.length !== fragment.type.length) attr.nullable = true; if (members.length > 0) attr.type = members[0]; diff --git a/resources/openApi.ts b/resources/openApi.ts index 15d1cd16ed..f4fd9b81ed 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -30,11 +30,15 @@ function toOpenApiDialect(fragment: JsonSchemaFragment): JsonSchemaFragment { ...rest } = fragment; const out: JsonSchemaFragment = rest; - // 3.0 has no `'null'` type and no type unions; nullability is the `nullable` keyword alone. + // 3.0 has no `'null'` type and no type unions: nullability is the `nullable` keyword, and a genuine + // multi-type union is `oneOf`. Keeping only the first member would silently narrow the contract. if (Array.isArray(out.type)) { const members = out.type.filter((t) => t !== 'null'); if (members.length !== out.type.length) out.nullable = true; - if (members.length > 0) out.type = members[0]; + if (members.length > 1) { + delete out.type; + out.oneOf = members.map((member) => ({ type: member })); + } else if (members.length === 1) out.type = members[0]; else delete out.type; } else if (out.type === 'null') { delete out.type; @@ -62,6 +66,24 @@ function toOpenApiDialect(fragment: JsonSchemaFragment): JsonSchemaFragment { return out; } +/** + * The non-null members of an attribute's declared type union, but only when there is more than one — + * `['string','null']` is nullability, not a union, and the existing single-type path already handles + * it. Returns undefined when the attribute has no union to translate. + */ +function unionMembers(attr: { types?: readonly string[] }): string[] | undefined { + if (!attr.types) return undefined; + const members = attr.types.filter((member) => member !== 'null'); + return members.length > 1 ? members : undefined; +} + +/** A single union member as a 3.0 Schema Object, using the same type mapping as the scalar path. */ +function openApiUnionMember(member: string) { + return !DATA_TYPES[member] && JSON_SCHEMA_SCALAR_TYPES.has(member) + ? new Type(member) + : new Type(DATA_TYPES[member], member); +} + const SCHEMA_COMP_REF = '#/components/schemas/'; const DESCRIPTION_200 = 'successful operation'; @@ -190,6 +212,7 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { if (attributes) { for (const attr of attributes) { const { type, name, elements, relationship, definition, nullable, description, hidden } = attr; + const union = unionMembers(attr); // @hidden field-level: suppress the attribute from props, query params, and required. if (hidden) continue; const def = definition ?? elements?.definition; @@ -217,6 +240,10 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { // Nested object from `static properties` — the shared projector emits the full object // schema (sub-properties recursed); OpenAPI's table path uses $refs instead. props[name] = toOpenApiDialect(attributeToFragment(attr)); + } else if (union) { + // A genuine multi-type union (`['string','number']`). 3.0 has no type arrays, so the + // equivalent is `oneOf`; `attr.type` alone would drop every member but the first. + props[name] = { oneOf: union.map(openApiUnionMember) }; } else if (type === 'array') { if (!elements) { // `{ type: 'array' }` with no items — valid JSON Schema (array of anything). diff --git a/unitTests/components/mcp/tools/convergence.test.js b/unitTests/components/mcp/tools/convergence.test.js index be971c0bc8..b507bf76a7 100644 --- a/unitTests/components/mcp/tools/convergence.test.js +++ b/unitTests/components/mcp/tools/convergence.test.js @@ -23,6 +23,7 @@ function makeResources() { Widget.properties = { id: { type: 'string', primaryKey: true }, label: { type: 'string', description: 'Human-readable label' }, + mixed: { type: ['string', 'number'] }, }; for (const v of ['get', 'put', 'patch', 'delete', 'search', 'post']) Widget.prototype[v] = function () {}; Widget.get = async (t) => ({ id: t.id }); @@ -74,4 +75,20 @@ describe('mcp/openapi — #1920 description convergence across surfaces', () => // Convergence: the per-property description is identical on both surfaces. assert.equal(create.inputSchema.properties.label.description, schema.properties.label.description); }); + + it('expresses a declared type union on each surface in that surface’s own dialect', () => { + const resources = makeResources(); + + _setResourcesForTest(resources); + registerApplicationTools(); + const create = getTool('create_Widget'); + // MCP speaks JSON Schema, which has type unions — pass the author's declaration through. + assert.deepEqual(create.inputSchema.properties.mixed.type, ['string', 'number']); + + // OpenAPI 3.0 has no type arrays; the equivalent is `oneOf`. Same declaration, two encodings — + // what must NOT happen is either surface narrowing it to `string`. + const schema = generateJsonApi(resources, 'https://harper.fast').components.schemas.Widget; + assert.deepEqual(schema.properties.mixed.oneOf, [{ type: 'string' }, { type: 'number' }]); + assert.equal(schema.properties.mixed.type, undefined); + }); }); diff --git a/unitTests/resources/graphqlMetadata.test.js b/unitTests/resources/graphqlMetadata.test.js index 5cec03f946..3285287fed 100644 --- a/unitTests/resources/graphqlMetadata.test.js +++ b/unitTests/resources/graphqlMetadata.test.js @@ -199,5 +199,24 @@ describe('GraphQL parser — metadata capture (#1095)', () => { assert.strictEqual(note.type, 'string'); assert.strictEqual(note.nullable, true, 'the null member must become nullable, not be dropped'); }); + + it('preserves a genuine multi-type union instead of keeping only the first member', () => { + const [mixed] = projectPropertiesToAttributes({ mixed: { type: ['string', 'number'] } }); + assert.deepStrictEqual(mixed.types, ['string', 'number']); + assert.strictEqual(mixed.type, 'string', 'single-type consumers still see the first member'); + }); + + it('round-trips a union back to the declared fragment (properties -> attributes -> properties)', () => { + const declared = { mixed: { type: ['string', 'number'] }, maybe: { type: ['string', 'null'] } }; + const round = projectAttributesToProperties(projectPropertiesToAttributes(declared)); + assert.deepStrictEqual(round.mixed.type, ['string', 'number']); + assert.deepStrictEqual(round.maybe.type, ['string', 'null']); + }); + + it('keeps a `["null"]`-only declaration nullable rather than silently untyped', () => { + const [nothing] = projectPropertiesToAttributes({ nothing: { type: ['null'] } }); + assert.strictEqual(nothing.nullable, true); + assert.deepStrictEqual(nothing.types, ['null']); + }); }); }); diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index aa8e821817..025ca81b5a 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -355,6 +355,8 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { kind: { type: 'string', const: 'widget' }, nothing: { type: 'null' }, maybe: { type: ['string', 'null'] }, + mixed: { type: ['string', 'number'] }, + mixedMaybe: { type: ['string', 'integer', 'null'] }, nested: { type: 'object', properties: { inner: { type: 'string', const: 'x' }, deepNull: { type: 'null' } } }, list: { type: 'array', items: { type: 'string', const: 'y' } }, nullableEnum: { type: 'string', enum: ['a', 'b'], nullable: true }, @@ -435,9 +437,34 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { expect(props.nullableConst.enum).to.deep.equal(['fixed', null]); }); + it('translates a genuine multi-type union to `oneOf`', () => { + // 3.0 has no type arrays. Keeping only the first member would narrow the contract silently — + // a client would be told `mixed` is a string when the resource also accepts a number. + const props = buildDocument().components.schemas.Widget.properties; + expect(props.mixed).to.deep.equal({ oneOf: [{ type: 'string' }, { type: 'number' }] }); + expect(props.mixed).to.not.have.property('type'); + }); + + it('carries nullability alongside a union', () => { + const props = buildDocument().components.schemas.Widget.properties; + expect(props.mixedMaybe.oneOf).to.deep.equal([{ type: 'string' }, { type: 'integer' }]); + expect(props.mixedMaybe.nullable).to.equal(true); + }); + it('emits the properties under test (guards the walk assertions against an empty document)', () => { const props = buildDocument().components.schemas.Widget.properties; - for (const key of ['kind', 'nothing', 'maybe', 'nested', 'list', 'nullableEnum', 'nullableConst', 'when']) { + for (const key of [ + 'kind', + 'nothing', + 'maybe', + 'mixed', + 'mixedMaybe', + 'nested', + 'list', + 'nullableEnum', + 'nullableConst', + 'when', + ]) { expect(props, `fixture property ${key} missing — walk assertions would pass vacuously`).to.have.property(key); } }); From eaa755b4824f44b1a7858d3e55debea10f850d67 Mon Sep 17 00:00:00 2001 From: Kyle Bernhardy Date: Mon, 27 Jul 2026 13:26:32 -0600 Subject: [PATCH 10/10] fix(openapi): express a null-only declaration at nested and array depth too The top-level scalar path translated a bare `type: 'null'`; the nested-object and array-item paths deleted the type and said nothing, so the same declaration described a different contract depending on where it appeared. All three now emit a null-only `enum` alongside `nullable`. 3.0 has no `'null'` type and a bare `nullable` on an untyped schema constrains nothing, so the enum is what actually carries the author's meaning. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0188G62J9fZQg4J9rVuqLzjy --- resources/openApi.ts | 12 ++++++++---- unitTests/resources/openApi.test.js | 10 ++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/resources/openApi.ts b/resources/openApi.ts index f4fd9b81ed..8119c5f1b7 100644 --- a/resources/openApi.ts +++ b/resources/openApi.ts @@ -41,7 +41,11 @@ function toOpenApiDialect(fragment: JsonSchemaFragment): JsonSchemaFragment { } else if (members.length === 1) out.type = members[0]; else delete out.type; } else if (out.type === 'null') { + // A null-only declaration. 3.0 has no `'null'` type, and a bare `nullable` on an untyped schema + // constrains nothing — a null-only `enum` is the form the dialect can actually express. delete out.type; + out.nullable = true; + if (out.enum === undefined) out.enum = [null]; } // `const` is draft-06; emit the equivalent single-value `enum`, intersecting when both are declared. if (constValue !== undefined) { @@ -256,7 +260,7 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { } else if (!DATA_TYPES[elements.type] && JSON_SCHEMA_SCALAR_TYPES.has(elements.type)) { props[name] = elements.type === 'null' - ? { type: 'array', items: {} } + ? { type: 'array', items: { nullable: true, enum: [null] } } : { type: 'array', items: new Type(elements.type) }; } else { props[name] = { type: 'array', items: new Type(DATA_TYPES[elements.type], elements.type) }; @@ -264,9 +268,9 @@ export function generateJsonApi(resources: Resources, serverHttpURL: string) { } else if (type === 'Any') { props[name] = { format: type }; } else if (!DATA_TYPES[type] && JSON_SCHEMA_SCALAR_TYPES.has(type)) { - // OpenAPI 3.0.3 has no `'null'` type — nullability is expressed by the `nullable` - // keyword, so a bare `type: 'null'` becomes an untyped nullable schema. - props[name] = type === 'null' ? { nullable: true } : new Type(type); + // OpenAPI 3.0.3 has no `'null'` type. A bare `nullable` on an untyped schema says + // nothing, so express "only null" the one way the dialect can: a null-only `enum`. + props[name] = type === 'null' ? { nullable: true, enum: [null] } : new Type(type); } else { props[name] = new Type(DATA_TYPES[type], type); } diff --git a/unitTests/resources/openApi.test.js b/unitTests/resources/openApi.test.js index 025ca81b5a..ec06e59be6 100644 --- a/unitTests/resources/openApi.test.js +++ b/unitTests/resources/openApi.test.js @@ -358,6 +358,7 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { mixed: { type: ['string', 'number'] }, mixedMaybe: { type: ['string', 'integer', 'null'] }, nested: { type: 'object', properties: { inner: { type: 'string', const: 'x' }, deepNull: { type: 'null' } } }, + nullList: { type: 'array', items: { type: 'null' } }, list: { type: 'array', items: { type: 'string', const: 'y' } }, nullableEnum: { type: 'string', enum: ['a', 'b'], nullable: true }, nullableConst: { type: 'string', const: 'fixed', nullable: true }, @@ -451,6 +452,15 @@ describe('openApi — declared dialect compliance (3.0.3)', () => { expect(props.mixedMaybe.nullable).to.equal(true); }); + it('expresses a null-only declaration at every depth, not just the top level', () => { + // 3.0 cannot say `type: 'null'`, but it can say "only null" with a null-only enum. The nested and + // array-item paths went through a different branch than the top-level one and said nothing at all. + const props = buildDocument().components.schemas.Widget.properties; + expect(props.nothing).to.deep.equal({ nullable: true, enum: [null] }); + expect(props.nested.properties.deepNull).to.deep.equal({ nullable: true, enum: [null] }); + expect(props.nullList.items).to.deep.equal({ nullable: true, enum: [null] }); + }); + it('emits the properties under test (guards the walk assertions against an empty document)', () => { const props = buildDocument().components.schemas.Widget.properties; for (const key of [