diff --git a/CHANGELOG.md b/CHANGELOG.md index cb877d6..e49812e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ All notable changes to this project will be documented in this file. This project follows [Keep a Changelog](https://keepachangelog.com/). +## [0.5.0] - 2026-04-16 + +Applies to both the Python and TypeScript libraries. The tool contract is now identical across both SDKs. + +### Changed +- **Breaking:** Auto-generated `create_{name}` and `update_{name}` MCP tools now take flat kwargs at the top level. The `{data: {...}}` wrapper has been removed — pass entity fields directly, e.g. `create_deal({"title": "...", "amount": 1000, "stage": "qualified"})`. Mixing the old and new shapes in the same tool list was measurably confusing LLMs and driving ~30% tool-call failure rates on the auto-generated CRUD surface. The flat form matches the hand-written tool convention and the FastMCP / MCP SDK idiom. +- **Breaking (TypeScript only):** `get_{name}`, `update_{name}`, and `delete_{name}` tools now take an entity-specific id parameter (e.g. `contact_id`, `deal_id`) instead of the generic `entity_id`. This matches the Python library and the existing relationship-tool convention. +- `get_{name}`, `update_{name}`, and `delete_{name}` tool schemas now include a JSON Schema `examples` field with a minimal valid call so LLMs have an in-context anchor for the correct shape. Author-supplied `examples` on the entity schema are passed through verbatim for `create_{name}` (base entity fields stripped so framework-managed values don't leak into tool examples). + +### Fixed +- `tools/list` no longer forces a network fetch of `https://upjack.dev/schemas/v1/upjack-entity.schema.json` when activities or any `allOf + $ref` schema is in play. The base-entity `$ref` is now inlined at schema-load time (`load_schema` / `loadSchema`). This eliminates a ~4-second-per-call penalty that hit every activity-enabled app. + +### Added +- `upjack.schema.BASE_ENTITY_REF` (Python and TypeScript) — the canonical `$id` / `$ref` URL for the bundled base entity schema, exported for consumers that want to recognise or rewrite it. The inlining itself is performed automatically by `load_schema` / `loadSchema` and is not part of the public API. +- `upjack.schema.BASE_ENTITY_MARKER` (TypeScript only) — the non-standard key (`x-upjack-base-entity: true`) attached to the inlined base-entity schema so downstream code can identify it without the `$id` that would otherwise conflict with AJV's pre-registered copy. + ## [0.3.1] - 2026-03-27 ### Fixed diff --git a/lib/python/pyproject.toml b/lib/python/pyproject.toml index 417d2d7..49db450 100644 --- a/lib/python/pyproject.toml +++ b/lib/python/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "upjack" -version = "0.4.4" +version = "0.5.0" description = "Schema-driven entity management for AI-native applications" readme = "README.md" license = {text = "Apache-2.0"} diff --git a/lib/python/src/upjack/__init__.py b/lib/python/src/upjack/__init__.py index 0bd6d52..362ec86 100644 --- a/lib/python/src/upjack/__init__.py +++ b/lib/python/src/upjack/__init__.py @@ -1,6 +1,6 @@ """NimbleBrain Upjack — schema-driven entity management for AI-native applications.""" -__version__ = "0.4.4" +__version__ = "0.5.0" from upjack.activity import ACTIVITY_ENTITY_DEF, get_activity_schema from upjack.app import UpjackApp diff --git a/lib/python/src/upjack/activity.py b/lib/python/src/upjack/activity.py index 435ebae..a40aae5 100644 --- a/lib/python/src/upjack/activity.py +++ b/lib/python/src/upjack/activity.py @@ -7,10 +7,11 @@ Opt-in via ``"activities": true`` in the manifest's upjack extension. """ -import json from pathlib import Path from typing import Any +from upjack.schema import load_schema + _SCHEMA_PATH = Path(__file__).parent / "schemas" / "activity.schema.json" ACTIVITY_ENTITY_DEF: dict[str, Any] = { @@ -22,5 +23,5 @@ def get_activity_schema() -> dict[str, Any]: - """Load the built-in activity schema from the package's schemas directory.""" - return json.loads(_SCHEMA_PATH.read_text()) + """Load the built-in activity schema (with base-entity $ref inlined).""" + return load_schema(_SCHEMA_PATH) diff --git a/lib/python/src/upjack/schema.py b/lib/python/src/upjack/schema.py index b2d0ef8..b152f70 100644 --- a/lib/python/src/upjack/schema.py +++ b/lib/python/src/upjack/schema.py @@ -14,46 +14,91 @@ _SCHEMAS_DIR = Path(__file__).parent / "schemas" -# Load the bundled base entity schema and build a registry so that -# app schemas using $ref to the remote URL resolve locally. +# Canonical $id / $ref URL for the bundled base entity schema. App schemas +# reference this via `allOf: [{"$ref": BASE_ENTITY_REF}]` so apps can layer +# their own fields on top of the framework-managed ones. +BASE_ENTITY_REF = "https://upjack.dev/schemas/v1/upjack-entity.schema.json" + +# Non-standard marker attached to the inlined base-entity schema so downstream +# code can identify it without the ``$ref`` or ``$id`` (both cause problems for +# JSON Schema validators that auto-register schemas by ``$id``). This matches +# the TypeScript SDK's convention for tool-schema portability. +BASE_ENTITY_MARKER = "x-upjack-base-entity" + +# Framework-managed base entity fields — the canonical set used across both +# SDKs. Stripped from tool input schemas (auto-managed, not user-controlled) +# and also filtered out of author-supplied examples so the published tool +# schema doesn't instruct LLMs to send them. +BASE_ENTITY_FIELDS = frozenset( + { + "id", + "type", + "version", + "created_at", + "updated_at", + "created_by", + "status", + "tags", + "source", + "relationships", + } +) + +# The bundled copy of the base entity schema, loaded once at import time. _BASE_SCHEMA = json.loads((_SCHEMAS_DIR / "upjack-entity.schema.json").read_text()) _BASE_RESOURCE = referencing.Resource.from_contents( _BASE_SCHEMA, default_specification=referencing.jsonschema.DRAFT202012 ) -_REGISTRY = referencing.Registry().with_resource( - "https://upjack.dev/schemas/v1/upjack-entity.schema.json", _BASE_RESOURCE -) +_REGISTRY = referencing.Registry().with_resource(BASE_ENTITY_REF, _BASE_RESOURCE) def load_schema(path: str | Path) -> dict[str, Any]: - """Load a JSON Schema from a file path. + """Load a JSON Schema from disk and inline the base-entity ``$ref``. + + Any ``allOf: [{"$ref": BASE_ENTITY_REF}]`` entry is replaced with the + bundled base-entity schema inline, so every downstream consumer sees a + fully self-contained schema. This is the single source of truth for + $ref resolution — no caller needs to do it again. + """ + schema = json.loads(Path(path).read_text()) + _inline_base_entity_ref(schema) + return schema - Args: - path: Path to the .schema.json file. - Returns: - Parsed JSON Schema as a dict. +def _inline_base_entity_ref(node: Any) -> None: + """Walk a schema in place, replacing every ``$ref: BASE_ENTITY_REF`` dict + with a deep copy of the bundled base schema contents. - Raises: - FileNotFoundError: If the schema file doesn't exist. - json.JSONDecodeError: If the file isn't valid JSON. + ``$schema`` and ``$id`` are dropped from the inlined copy — the shared + ``$id`` would clash with validator registries that auto-register schemas + by identifier (this is what bites AJV on the TypeScript side). A + non-standard ``BASE_ENTITY_MARKER`` key is attached so downstream code + can still identify the inlined base without those identifiers. Both SDKs + use this convention so tool schemas published over MCP are byte-aligned. """ - path = Path(path) - return json.loads(path.read_text()) + if isinstance(node, dict): + all_of = node.get("allOf") + if isinstance(all_of, list): + for i, sub in enumerate(all_of): + if isinstance(sub, dict) and sub.get("$ref") == BASE_ENTITY_REF: + inlined = copy.deepcopy(_BASE_SCHEMA) + inlined.pop("$schema", None) + inlined.pop("$id", None) + inlined[BASE_ENTITY_MARKER] = True + all_of[i] = inlined + for value in node.values(): + _inline_base_entity_ref(value) + elif isinstance(node, list): + for item in node: + _inline_base_entity_ref(item) def validate_entity(data: dict[str, Any], schema: dict[str, Any]) -> None: """Validate entity data against a JSON Schema. - Uses JSON Schema draft 2020-12 validation. Resolves $ref to the - base entity schema via a bundled local copy. - - Args: - data: Entity data to validate. - schema: JSON Schema to validate against. - - Raises: - jsonschema.ValidationError: If validation fails. + Uses JSON Schema draft 2020-12 validation. The registry resolves any + remaining ``$ref`` to the base entity schema locally, so validation works + even if the caller handed us a schema that bypassed ``load_schema``. """ missing = _check_required_without_defaults(schema) for field in missing: @@ -68,19 +113,12 @@ def validate_entity(data: dict[str, Any], schema: dict[str, Any]) -> None: def hydrate_defaults(data: dict[str, Any], schema: dict[str, Any]) -> dict[str, Any]: - """Fill missing fields in data with defaults from the schema. + """Fill missing fields in ``data`` with defaults from ``schema``. - Walks the schema's "properties" (and any allOf members) to find - fields with a "default" value. If the field is absent from data, - sets it to the default. Operates on a shallow copy — does not - mutate the input dict. - - Args: - data: Entity data (may be missing fields). - schema: JSON Schema with optional "default" values on properties. - - Returns: - A new dict with missing fields filled from schema defaults. + Walks the schema's ``properties`` and any ``allOf`` members. Assumes + ``schema`` has been loaded via :func:`load_schema` (so any base-entity + ``$ref`` has been inlined) — does not resolve live ``$ref`` values. + Operates on a shallow copy of ``data``. """ result = dict(data) _apply_property_defaults(result, schema) @@ -88,14 +126,9 @@ def hydrate_defaults(data: dict[str, Any], schema: dict[str, Any]) -> dict[str, def _apply_property_defaults(data: dict[str, Any], schema: dict[str, Any]) -> None: - """Apply defaults from a single schema node's properties.""" - # Handle allOf — walk each sub-schema + """Apply defaults from a single schema node's properties and allOf members.""" for sub in schema.get("allOf", []): - # Resolve $ref to the base entity schema - ref = sub.get("$ref") - if ref and ref in _REF_MAP: - _apply_property_defaults(data, _REF_MAP[ref]) - else: + if isinstance(sub, dict): _apply_property_defaults(data, sub) props = schema.get("properties", {}) @@ -104,25 +137,12 @@ def _apply_property_defaults(data: dict[str, Any], schema: dict[str, Any]) -> No data[field_name] = copy.deepcopy(field_schema["default"]) -# Map $ref URIs to resolved schemas for hydration -_REF_MAP: dict[str, dict[str, Any]] = { - "https://upjack.dev/schemas/v1/upjack-entity.schema.json": _BASE_SCHEMA, -} - - def resolve_entity_schema( base_schema: dict[str, Any], app_schema: dict[str, Any] ) -> dict[str, Any]: - """Create a composed schema from base entity schema and app-specific schema. + """Create a composed schema from the base entity schema and an app schema. - Uses allOf composition so both base and app constraints apply. - - Args: - base_schema: The upjack-entity base schema. - app_schema: The app-specific entity schema. - - Returns: - Composed schema with allOf referencing both. + Uses ``allOf`` composition so both base and app constraints apply. """ return { "$schema": "https://json-schema.org/draft/2020-12/schema", @@ -217,25 +237,21 @@ def validate_schema_change( def build_entity_output_schema(schema: dict[str, Any]) -> dict[str, Any]: """Build an output schema for a single-entity tool response. - Returns the full entity schema (including base fields) with JSON Schema - meta keywords stripped, suitable for use as a tool's ``outputSchema``. - MCP requires ``type: "object"`` on every outputSchema. + Expects ``schema`` to be already self-contained (loaded via + :func:`load_schema`). Strips JSON Schema meta keywords that don't belong + in a tool output schema. MCP requires ``type: "object"`` on every + outputSchema. """ result = copy.deepcopy(schema) result.pop("$schema", None) result.pop("$id", None) - # MCP spec requires outputSchema to have type: "object" if "type" not in result: result["type"] = "object" return result def build_list_output_schema(entity_schema: dict[str, Any]) -> dict[str, Any]: - """Build an output schema for a list/search response envelope. - - Returns an object schema with ``entities`` (array of entity schemas) - and ``count`` (integer). - """ + """Build an output schema for a list/search response envelope.""" item_schema = build_entity_output_schema(entity_schema) return { "type": "object", diff --git a/lib/python/src/upjack/server.py b/lib/python/src/upjack/server.py index 6d7b0bf..a7cb34b 100644 --- a/lib/python/src/upjack/server.py +++ b/lib/python/src/upjack/server.py @@ -22,26 +22,19 @@ from upjack.app import UpjackApp from upjack.relations import rebuild_index from upjack.schema import ( + BASE_ENTITY_FIELDS, + BASE_ENTITY_MARKER, + BASE_ENTITY_REF, build_entity_output_schema, build_list_output_schema, load_schema, validate_schema_change, ) -# Base entity fields auto-managed by the framework — stripped from tool input schemas -_BASE_ENTITY_KEYS = frozenset( - { - "id", - "type", - "version", - "created_at", - "updated_at", - "created_by", - "status", - "tags", - "source", - } -) +# Alias for readability at call sites — this is the canonical set of +# framework-managed fields, imported from upjack.schema so both the server +# module here and the parity tests reference one source of truth. +_BASE_ENTITY_KEYS = BASE_ENTITY_FIELDS def _wrap_list(entities: list[dict[str, Any]], **extra: Any) -> dict[str, Any]: @@ -55,25 +48,39 @@ def _wrap_list(entities: list[dict[str, Any]], **extra: Any) -> dict[str, Any]: def _prepare_entity_schema(schema: dict[str, Any], *, for_update: bool = False) -> dict[str, Any]: - """Prepare an entity JSON Schema for use as an MCP tool input. - - Strips base entity fields (auto-managed by the framework) and JSON Schema - meta keywords that don't belong in a tool input schema. For update tools, - removes ``required`` since updates are partial merges. + """Project an app entity schema onto an MCP tool input schema. + + Tool inputs carry only user-controlled fields — framework-managed base + fields (id, type, timestamps, status, tags, source) are excluded. That + means: + + - Any ``allOf`` member pointing at the bundled base entity schema is + dropped entirely — it only contributes base fields. + - ``properties`` and ``required`` are filtered to remove those same base + fields. + - For update tools, ``required`` is dropped (partial merge semantics). + - JSON Schema meta keywords (``$schema``, ``$id``) are stripped. + + Assumes ``schema`` has been loaded via ``load_schema`` so any base-entity + ``$ref`` has already been inlined. Allowing a raw on-disk schema through + here would leave a remote ``$ref`` that downstream serializers could try + to dereference over the network. """ result = copy.deepcopy(schema) - - # Strip JSON Schema meta keywords not applicable inside tool input result.pop("$schema", None) result.pop("$id", None) + if "allOf" in result: + result["allOf"] = [sub for sub in result["allOf"] if not _is_base_entity_schema(sub)] + if not result["allOf"]: + del result["allOf"] + if "properties" in result: result["properties"] = { k: v for k, v in result["properties"].items() if k not in _BASE_ENTITY_KEYS } if for_update: - # Updates are partial merges — all fields optional result.pop("required", None) elif "required" in result: result["required"] = [r for r in result["required"] if r not in _BASE_ENTITY_KEYS] @@ -83,6 +90,15 @@ def _prepare_entity_schema(schema: dict[str, Any], *, for_update: bool = False) return result +def _is_base_entity_schema(node: Any) -> bool: + """True if ``node`` is either a ``$ref`` to the base entity schema (raw + on-disk form) or an inlined copy carrying ``BASE_ENTITY_MARKER`` (the form + produced by ``load_schema``).""" + if not isinstance(node, dict): + return False + return node.get("$ref") == BASE_ENTITY_REF or node.get(BASE_ENTITY_MARKER) is True + + def _make_entity_tool( *, name: str, @@ -181,23 +197,34 @@ def _register_entity_tools( entity_out = build_entity_output_schema(schema) if schema else None list_out = build_list_output_schema(schema) if schema else None + # Author-supplied examples on the entity schema are passed through to + # create / update tool schemas as in-context anchors for LLMs. This is + # pass-through, not generation — we don't invent values. Base entity + # fields are auto-managed, so we strip them from examples even if the + # author included them. + schema_examples = schema.get("examples") if isinstance(schema, dict) else None + raw_examples = schema_examples if isinstance(schema_examples, list) else [] + author_examples = [ + {k: v for k, v in ex.items() if k not in _BASE_ENTITY_KEYS} + for ex in raw_examples + if isinstance(ex, dict) + ] + # --- create_{name} --- - # Use the entity's JSON Schema so LLMs see full field structure if schema: - data_schema = _prepare_entity_schema(schema) + create_params = _prepare_entity_schema(schema) else: - data_schema = {"type": "object"} + create_params = {"type": "object"} + create_params.setdefault("type", "object") + if author_examples: + create_params["examples"] = copy.deepcopy(author_examples) mcp.add_tool( _make_entity_tool( name=f"create_{name}", description=f"Create a new {name}. {id_hint}.", - parameters={ - "type": "object", - "properties": {"data": data_schema}, - "required": ["data"], - }, - handler=lambda args, _n=name: app.create_entity(_n, args["data"]), + parameters=create_params, + handler=lambda args, _n=name: app.create_entity(_n, args), output_schema=entity_out, ) ) @@ -216,6 +243,7 @@ def _register_entity_tools( }, }, "required": [id_param], + "examples": [{id_param: f"{prefix}_01HXXX"}], }, handler=lambda args, _n=name, _p=id_param: app.get_entity(_n, args[_p]), output_schema=entity_out, @@ -223,29 +251,47 @@ def _register_entity_tools( ) # --- update_{name} --- - # Use the entity's JSON Schema with required stripped (partial merge) if schema: - update_data_schema = _prepare_entity_schema(schema, for_update=True) + update_fields_schema = _prepare_entity_schema(schema, for_update=True) else: - update_data_schema = {"type": "object"} + update_fields_schema = {"type": "object"} + + raw_props = update_fields_schema.get("properties") + extra_props: dict[str, Any] = raw_props if isinstance(raw_props, dict) else {} + update_params: dict[str, Any] = { + "type": "object", + "properties": { + id_param: { + "type": "string", + "description": f"{name} ID ({prefix}_...)", + }, + **extra_props, + }, + "required": [id_param], + } + if author_examples: + update_params["examples"] = [ + {id_param: f"{prefix}_01HXXX", **example} for example in author_examples + ] mcp.add_tool( _make_entity_tool( name=f"update_{name}", - description=f"Update a {name} by ID. Merges fields by default. {id_hint}.", - parameters={ - "type": "object", - "properties": { - id_param: { - "type": "string", - "description": f"{name} ID ({prefix}_...)", - }, - "data": update_data_schema, - }, - "required": [id_param, "data"], - }, + description=( + f"Update a {name} by ID. Merges fields by default — pass any subset " + f"of fields to change. Unknown fields are merged onto the entity " + f"as-is (the schema does not enforce additionalProperties=false). " + f"{id_hint}." + ), + parameters=update_params, + # NB: we strip the id param from the payload before merging. + # If an app ever declares a top-level entity property literally + # named ``{entity_name}_id`` (e.g. ``user_id`` on a ``user`` + # entity), it becomes unreachable via the update tool. Avoid + # that collision in your schema — or use a differently-named + # external-id field. handler=lambda args, _n=name, _p=id_param: app.update_entity( - _n, args[_p], args["data"] + _n, args[_p], {k: v for k, v in args.items() if k != _p} ), output_schema=entity_out, ) @@ -302,6 +348,7 @@ def search_tool( }, }, "required": [id_param], + "examples": [{id_param: f"{prefix}_01HXXX"}], }, handler=lambda args, _n=name, _p=id_param: app.delete_entity( _n, args[_p], hard=args.get("hard", False) @@ -377,19 +424,9 @@ def seed_data() -> dict[str, Any]: _FIELD_NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") -_BASE_ENTITY_FIELD_NAMES = frozenset( - { - "id", - "type", - "version", - "created_at", - "updated_at", - "created_by", - "status", - "tags", - "relationships", - } -) +# Reserved field names for the add_field tool — any name matching one of the +# framework-managed fields is rejected. Same set as the tool-input strip list. +_BASE_ENTITY_FIELD_NAMES = BASE_ENTITY_FIELDS def _register_add_field_tool( diff --git a/lib/python/tests/test_activity.py b/lib/python/tests/test_activity.py index e81a42c..c4b3593 100644 --- a/lib/python/tests/test_activity.py +++ b/lib/python/tests/test_activity.py @@ -7,6 +7,7 @@ from upjack.activity import ACTIVITY_ENTITY_DEF, get_activity_schema from upjack.app import UpjackApp +from upjack.schema import BASE_ENTITY_MARKER NAMESPACE = "apps/test" ENTITIES = [ @@ -41,11 +42,22 @@ def test_schema_detail_is_optional_with_default(self): assert detail["type"] == "object" assert detail["default"] == {} - def test_schema_uses_allof_ref(self): + def test_schema_inlines_base_entity(self): + """get_activity_schema() returns a fully self-contained schema — the + base entity $ref is inlined at load time, so no downstream consumer + needs to dereference it over the network.""" schema = get_activity_schema() assert "allOf" in schema - refs = [entry.get("$ref") for entry in schema["allOf"]] - assert "https://upjack.dev/schemas/v1/upjack-entity.schema.json" in refs + # No unresolved $refs and no leftover $id (both trip up validators + # that auto-register schemas by identifier). + for entry in schema["allOf"]: + assert "$ref" not in entry, f"Unresolved $ref in allOf: {entry.get('$ref')}" + assert "$id" not in entry, f"Stray $id in inlined allOf: {entry.get('$id')}" + # The inlined base member carries our non-standard marker + base = next((e for e in schema["allOf"] if e.get(BASE_ENTITY_MARKER) is True), None) + assert base is not None, "base entity schema not inlined into allOf" + assert "id" in base["properties"] + assert "created_at" in base["properties"] class TestActivityEntityDef: diff --git a/lib/python/tests/test_examples.py b/lib/python/tests/test_examples.py index 3bc3c8e..af58e25 100644 --- a/lib/python/tests/test_examples.py +++ b/lib/python/tests/test_examples.py @@ -461,9 +461,7 @@ def test_create_contact_through_tool(self, mcp): _call_tool( mcp, "create_contact", - { - "data": {"first_name": "Sarah", "last_name": "Chen"}, - }, + {"first_name": "Sarah", "last_name": "Chen"}, ) ) assert result["id"].startswith("ct_") @@ -755,9 +753,7 @@ def test_create_task_through_tool(self, mcp): _call_tool( mcp, "create_task", - { - "data": {"title": "Buy groceries"}, - }, + {"title": "Buy groceries"}, ) ) assert result["id"].startswith("tsk_") @@ -813,7 +809,7 @@ def test_add_field_to_contact(self, mcp): _call_tool( mcp, "create_contact", - {"data": {"first_name": "Alice", "last_name": "Chen"}}, + {"first_name": "Alice", "last_name": "Chen"}, ) ) @@ -844,7 +840,7 @@ def test_hydrate_on_read_list(self, mcp): _call_tool( mcp, "create_contact", - {"data": {"first_name": "Bob", "last_name": "Smith"}}, + {"first_name": "Bob", "last_name": "Smith"}, ) ) @@ -871,7 +867,7 @@ def test_hydrate_on_read_search(self, mcp): _call_tool( mcp, "create_contact", - {"data": {"first_name": "Charlie", "last_name": "Davis"}}, + {"first_name": "Charlie", "last_name": "Davis"}, ) ) @@ -912,7 +908,7 @@ def test_add_field_to_task(self, mcp): _call_tool( mcp, "create_task", - {"data": {"title": "Write tests"}}, + {"title": "Write tests"}, ) ) @@ -954,7 +950,7 @@ def test_add_field_to_topic(self, mcp): _call_tool( mcp, "create_topic", - {"data": {"title": "AI Safety"}}, + {"title": "AI Safety"}, ) ) @@ -1097,19 +1093,15 @@ def mcp(self, tmp_path): return create_server(app_dir / "manifest.json", root=workspace) def test_query_deals_by_relationship_tool(self, mcp): - company = _run( - _call_tool(mcp, "create_company", {"data": {"name": "Acme", "industry": "Tech"}}) - ) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme", "industry": "Tech"})) deal = _run( _call_tool( mcp, "create_deal", { - "data": { - "title": "Big Deal", - "stage": "new", - "relationships": [{"rel": "company", "target": company["id"]}], - } + "title": "Big Deal", + "stage": "new", + "relationships": [{"rel": "company", "target": company["id"]}], }, ) ) @@ -1125,19 +1117,15 @@ def test_query_deals_by_relationship_tool(self, mcp): assert result["entities"][0]["id"] == deal["id"] def test_get_deal_composite_tool(self, mcp): - company = _run( - _call_tool(mcp, "create_company", {"data": {"name": "Acme", "industry": "Tech"}}) - ) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme", "industry": "Tech"})) contact = _run( _call_tool( mcp, "create_contact", { - "data": { - "first_name": "Sarah", - "last_name": "Chen", - "relationships": [{"rel": "works_at", "target": company["id"]}], - } + "first_name": "Sarah", + "last_name": "Chen", + "relationships": [{"rel": "works_at", "target": company["id"]}], }, ) ) @@ -1146,15 +1134,13 @@ def test_get_deal_composite_tool(self, mcp): mcp, "create_deal", { - "data": { - "title": "Agent Platform", - "stage": "qualification", - "value": 48000, - "relationships": [ - {"rel": "primary_contact", "target": contact["id"]}, - {"rel": "company", "target": company["id"]}, - ], - } + "title": "Agent Platform", + "stage": "qualification", + "value": 48000, + "relationships": [ + {"rel": "primary_contact", "target": contact["id"]}, + {"rel": "company", "target": company["id"]}, + ], }, ) ) @@ -1164,7 +1150,7 @@ def test_get_deal_composite_tool(self, mcp): assert "company" in result["_related"] def test_rebuild_index_tool(self, mcp): - _run(_call_tool(mcp, "create_company", {"data": {"name": "Acme", "industry": "Tech"}})) + _run(_call_tool(mcp, "create_company", {"name": "Acme", "industry": "Tech"})) result = _run(_call_tool(mcp, "rebuild_index", {})) assert result["success"] is True diff --git a/lib/python/tests/test_schema.py b/lib/python/tests/test_schema.py index 725c6b4..b68f4ad 100644 --- a/lib/python/tests/test_schema.py +++ b/lib/python/tests/test_schema.py @@ -17,6 +17,33 @@ ) +class TestSdkParity: + """Pin the cross-SDK contract so Python and TypeScript stay aligned. + + The TS SDK has a mirror test (see ``lib/typescript/tests/schema.test.ts`` + ``SDK parity``) asserting the same canonical set. If you change one, + change the other — and update CHANGELOG to note the tool-schema delta. + """ + + def test_base_entity_fields_canonical_set(self): + from upjack.schema import BASE_ENTITY_FIELDS + + assert BASE_ENTITY_FIELDS == frozenset( + { + "id", + "type", + "version", + "created_at", + "updated_at", + "created_by", + "status", + "tags", + "source", + "relationships", + } + ) + + class TestLoadSchema: def test_loads_valid_schema(self, tmp_path): schema = {"type": "object", "properties": {"name": {"type": "string"}}} @@ -99,14 +126,26 @@ def test_does_not_mutate_input(self): hydrate_defaults(data, schema) assert "priority" not in data - def test_handles_allof_with_ref(self): - """Schemas using allOf with $ref to base entity schema.""" - schema = { - "allOf": [{"$ref": "https://upjack.dev/schemas/v1/upjack-entity.schema.json"}], - "properties": { - "score": {"type": "integer", "default": 0}, - }, - } + def test_handles_inlined_allof_base_schema(self, tmp_path): + """Schemas loaded via load_schema() carry the base entity inlined under + allOf — hydrate_defaults walks those inlined members to pull defaults + (tags, relationships, etc.) alongside app-level defaults.""" + import json as _json + + from upjack.schema import load_schema + + schema_path = tmp_path / "contact.schema.json" + schema_path.write_text( + _json.dumps( + { + "allOf": [{"$ref": "https://upjack.dev/schemas/v1/upjack-entity.schema.json"}], + "properties": { + "score": {"type": "integer", "default": 0}, + }, + } + ) + ) + schema = load_schema(schema_path) data = {"id": "ct_01JKXM9V3QWERTY123456ABCDF", "type": "contact"} result = hydrate_defaults(data, schema) # App-level default applied diff --git a/lib/python/tests/test_server.py b/lib/python/tests/test_server.py index 193fb1a..b8138af 100644 --- a/lib/python/tests/test_server.py +++ b/lib/python/tests/test_server.py @@ -430,10 +430,10 @@ def test_uses_title_when_display_name_missing(self, tmp_path): class TestToolInputSchemas: - """Verify that create/update tools expose full entity JSON Schema.""" + """Verify that create/update tools expose the entity JSON Schema flat (no `data` wrapper).""" def test_create_tool_exposes_entity_schema(self, tmp_path): - """create_* tools should have the entity schema nested under data.""" + """create_* tools should expose entity fields at the top level.""" entity_schema = { "$schema": "https://json-schema.org/draft/2020-12/schema", "type": "object", @@ -467,26 +467,26 @@ def test_create_tool_exposes_entity_schema(self, tmp_path): mcp = create_server(manifest_path, root=tmp_path / "workspace") input_schema = _run(_get_tool_input_schema(mcp, "create_campaign")) - # data property should contain the entity schema (minus base fields) - data_schema = input_schema["properties"]["data"] - assert "name" in data_schema["properties"] - assert data_schema["properties"]["name"]["description"] == "Campaign name" - assert "score" in data_schema["properties"] - assert data_schema["properties"]["score"]["minimum"] == 0 + # Fields are top-level — no `data` wrapper + assert "data" not in input_schema["properties"] + assert "name" in input_schema["properties"] + assert input_schema["properties"]["name"]["description"] == "Campaign name" + assert "score" in input_schema["properties"] + assert input_schema["properties"]["score"]["minimum"] == 0 # Nested structure preserved - assert "emotional_drivers" in data_schema["properties"] - fear = data_schema["properties"]["emotional_drivers"]["properties"]["fear"] + assert "emotional_drivers" in input_schema["properties"] + fear = input_schema["properties"]["emotional_drivers"]["properties"]["fear"] assert "theme" in fear["properties"] # Base fields stripped - assert "id" not in data_schema["properties"] - assert "type" not in data_schema["properties"] + assert "id" not in input_schema["properties"] + assert "type" not in input_schema["properties"] # $schema meta keyword stripped - assert "$schema" not in data_schema + assert "$schema" not in input_schema # Required preserved (minus base fields) - assert data_schema["required"] == ["name"] + assert input_schema["required"] == ["name"] - def test_update_tool_has_no_required_in_data(self, tmp_path): - """update_* tools should have all data fields optional (partial merge).""" + def test_update_tool_has_flat_fields_and_id(self, tmp_path): + """update_* tools should have the id and all entity fields flat at the top level.""" entity_schema = { "$schema": "https://json-schema.org/draft/2020-12/schema", "type": "object", @@ -512,15 +512,13 @@ def test_update_tool_has_no_required_in_data(self, tmp_path): mcp = create_server(manifest_path, root=tmp_path / "workspace") input_schema = _run(_get_tool_input_schema(mcp, "update_item")) - # Top-level requires item_id and data + # Flat top-level: item_id (required), name, score (all optional) + assert "data" not in input_schema["properties"] assert "item_id" in input_schema["properties"] - assert set(input_schema["required"]) == {"item_id", "data"} - # Data schema has no required (partial update) - data_schema = input_schema["properties"]["data"] - assert "required" not in data_schema - # But fields are still described - assert "name" in data_schema["properties"] - assert "score" in data_schema["properties"] + assert "name" in input_schema["properties"] + assert "score" in input_schema["properties"] + # Only the id is required (partial merge for everything else) + assert input_schema["required"] == ["item_id"] def test_create_tool_falls_back_to_opaque_without_schema(self, tmp_path): """Without a schema, create_* should still work with opaque object.""" @@ -546,7 +544,132 @@ def test_create_tool_falls_back_to_opaque_without_schema(self, tmp_path): ) mcp = create_server(manifest_path, root=tmp_path / "workspace") input_schema = _run(_get_tool_input_schema(mcp, "create_thing")) - assert input_schema["properties"]["data"]["type"] == "object" + # No schema → opaque object (no properties), still no `data` wrapper + assert input_schema["type"] == "object" + assert "data" not in input_schema.get("properties", {}) + + def test_create_tool_passes_through_schema_examples(self, tmp_path): + """create_* publishes ``examples`` from the entity schema verbatim. + + Authors own the examples — no heuristic invents values. If the schema + has no examples, the tool schema has none either. + """ + entity_schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "title": {"type": "string"}, + "amount": {"type": "integer"}, + "stage": {"type": "string"}, + }, + "required": ["title", "amount", "stage"], + "examples": [ + {"title": "Acme Q2 pilot", "amount": 25000, "stage": "qualified"}, + ], + } + manifest_path = _make_manifest( + tmp_path, + [{"name": "deal", "plural": "deals", "prefix": "dl"}], + ) + (tmp_path / "schemas" / "deal.schema.json").write_text(json.dumps(entity_schema)) + mcp = create_server(manifest_path, root=tmp_path / "workspace") + input_schema = _run(_get_tool_input_schema(mcp, "create_deal")) + + assert input_schema["examples"] == [ + {"title": "Acme Q2 pilot", "amount": 25000, "stage": "qualified"}, + ] + + def test_author_examples_strip_framework_managed_fields(self, tmp_path): + """Base entity fields (id, type, created_at, etc.) in author examples + must NOT leak into the published tool schema — framework manages those, + and an LLM would incorrectly try to send them. + """ + entity_schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": {"title": {"type": "string"}}, + "required": ["title"], + "examples": [ + { + "id": "dl_01HXXX", + "type": "deal", + "created_at": "2026-04-16T00:00:00Z", + "status": "active", + "tags": ["pilot"], + "title": "Acme Q2 pilot", + } + ], + } + manifest_path = _make_manifest( + tmp_path, + [{"name": "deal", "plural": "deals", "prefix": "dl"}], + ) + (tmp_path / "schemas" / "deal.schema.json").write_text(json.dumps(entity_schema)) + mcp = create_server(manifest_path, root=tmp_path / "workspace") + + create_schema = _run(_get_tool_input_schema(mcp, "create_deal")) + assert create_schema["examples"] == [{"title": "Acme Q2 pilot"}] + + update_schema = _run(_get_tool_input_schema(mcp, "update_deal")) + assert update_schema["examples"] == [ + {"deal_id": "dl_01HXXX", "title": "Acme Q2 pilot"}, + ] + + def test_create_tool_omits_examples_when_schema_has_none(self, tmp_path): + """No heuristic fills in when the author hasn't provided examples.""" + entity_schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": {"title": {"type": "string"}}, + "required": ["title"], + } + manifest_path = _make_manifest( + tmp_path, + [{"name": "deal", "plural": "deals", "prefix": "dl"}], + ) + (tmp_path / "schemas" / "deal.schema.json").write_text(json.dumps(entity_schema)) + mcp = create_server(manifest_path, root=tmp_path / "workspace") + input_schema = _run(_get_tool_input_schema(mcp, "create_deal")) + + assert "examples" not in input_schema + + def test_update_prepends_id_to_schema_examples(self, tmp_path): + """update_* examples are derived by prepending the entity id to each + schema example — keeps the author's data shape and adds what the tool + needs to identify the record.""" + entity_schema = { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": {"title": {"type": "string"}}, + "required": ["title"], + "examples": [{"title": "Acme Q2 pilot"}], + } + manifest_path = _make_manifest( + tmp_path, + [{"name": "deal", "plural": "deals", "prefix": "dl"}], + ) + (tmp_path / "schemas" / "deal.schema.json").write_text(json.dumps(entity_schema)) + mcp = create_server(manifest_path, root=tmp_path / "workspace") + update_schema = _run(_get_tool_input_schema(mcp, "update_deal")) + + assert update_schema["examples"] == [ + {"deal_id": "dl_01HXXX", "title": "Acme Q2 pilot"}, + ] + + def test_get_and_delete_tools_always_include_id_examples(self, tmp_path): + """id-only tools always publish a canonical id example — no author + input required because the shape is trivial.""" + manifest_path = _make_manifest( + tmp_path, + [{"name": "deal", "plural": "deals", "prefix": "dl"}], + ) + mcp = create_server(manifest_path, root=tmp_path / "workspace") + + get_schema = _run(_get_tool_input_schema(mcp, "get_deal")) + assert get_schema["examples"] == [{"deal_id": "dl_01HXXX"}] + + delete_schema = _run(_get_tool_input_schema(mcp, "delete_deal")) + assert delete_schema["examples"] == [{"deal_id": "dl_01HXXX"}] class TestToolOutputSchemas: @@ -626,7 +749,7 @@ def test_structured_content_in_response(self, mcp): async def check(): async with Client(mcp) as client: - result = await client.call_tool("create_item", {"data": {"name": "Test"}}) + result = await client.call_tool("create_item", {"name": "Test"}) # structuredContent is set on CallToolResult assert result.structured_content is not None assert result.structured_content["name"] == "Test" @@ -651,7 +774,7 @@ def mcp(self, tmp_path): return create_server(manifest_path, root=workspace) def test_create_and_get_roundtrip(self, mcp): - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "Widget"}})) + created = _run(_call_tool(mcp, "create_item", {"name": "Widget"})) assert created["id"].startswith("it_") assert created["name"] == "Widget" assert created["type"] == "item" @@ -661,14 +784,15 @@ def test_create_and_get_roundtrip(self, mcp): assert fetched["name"] == "Widget" def test_update_merges_fields(self, mcp): - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "Old"}})) + created = _run(_call_tool(mcp, "create_item", {"name": "Old"})) updated = _run( _call_tool( mcp, "update_item", { "item_id": created["id"], - "data": {"name": "New", "extra": "field"}, + "name": "New", + "extra": "field", }, ) ) @@ -676,8 +800,8 @@ def test_update_merges_fields(self, mcp): assert updated["extra"] == "field" def test_list_returns_created_entities(self, mcp): - _run(_call_tool(mcp, "create_item", {"data": {"name": "A"}})) - _run(_call_tool(mcp, "create_item", {"data": {"name": "B"}})) + _run(_call_tool(mcp, "create_item", {"name": "A"})) + _run(_call_tool(mcp, "create_item", {"name": "B"})) result = _run(_call_tool(mcp, "list_items", {})) assert result["count"] == 2 @@ -686,8 +810,8 @@ def test_list_returns_created_entities(self, mcp): assert "limit" in result def test_search_finds_by_text(self, mcp): - _run(_call_tool(mcp, "create_item", {"data": {"name": "Alpha"}})) - _run(_call_tool(mcp, "create_item", {"data": {"name": "Beta"}})) + _run(_call_tool(mcp, "create_item", {"name": "Alpha"})) + _run(_call_tool(mcp, "create_item", {"name": "Beta"})) result = _run(_call_tool(mcp, "search_items", {"query": "Alpha"})) assert result["count"] == 1 @@ -696,7 +820,7 @@ def test_search_finds_by_text(self, mcp): assert "limit" in result def test_delete_soft(self, mcp): - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "Doomed"}})) + created = _run(_call_tool(mcp, "create_item", {"name": "Doomed"})) result = _run(_call_tool(mcp, "delete_item", {"item_id": created["id"]})) assert result["status"] == "deleted" @@ -705,7 +829,7 @@ def test_delete_soft(self, mcp): assert result["count"] == 0 def test_delete_hard(self, mcp): - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "Gone"}})) + created = _run(_call_tool(mcp, "create_item", {"name": "Gone"})) _run(_call_tool(mcp, "delete_item", {"item_id": created["id"], "hard": True})) # Hard-deleted entities are gone from disk entirely @@ -721,8 +845,8 @@ def test_delete_hard(self, mcp): class TestJsonStringDeserialization: """Raw Tool subclasses bypass FastMCP's Pydantic deserialization. - Over stdio transport, object arguments may arrive as JSON strings instead - of parsed dicts. The server must handle both forms. + Over stdio transport, object/array arguments may arrive as JSON strings + instead of parsed dicts/lists. The server must handle both forms. """ @pytest.fixture @@ -735,38 +859,73 @@ def mcp(self, tmp_path): workspace.mkdir() return create_server(manifest_path, root=workspace) - def test_create_with_data_as_json_string(self, mcp): - """create_* should work when data arrives as a JSON string.""" - data_str = json.dumps({"name": "StringWidget"}) - created = _run(_call_tool(mcp, "create_item", {"data": data_str})) - assert created["name"] == "StringWidget" - assert created["id"].startswith("it_") - - def test_update_with_data_as_json_string(self, mcp): - """update_* should work when data arrives as a JSON string.""" - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "Original"}})) - data_str = json.dumps({"name": "Updated"}) + def test_create_with_array_arg_as_json_string(self, mcp): + """create_* should parse array arguments that arrive as JSON strings.""" + rels_str = json.dumps([{"rel": "belongs_to", "target": "it_abc"}]) + created = _run( + _call_tool(mcp, "create_item", {"name": "Widget", "relationships": rels_str}) + ) + assert created["name"] == "Widget" + # The string should have been parsed into an actual list of dicts + assert isinstance(created.get("relationships"), list) + assert created["relationships"][0]["rel"] == "belongs_to" + + def test_update_with_object_arg_as_json_string(self, mcp): + """update_* should parse object arguments that arrive as JSON strings.""" + created = _run(_call_tool(mcp, "create_item", {"name": "Original"})) + detail_str = json.dumps({"nested": "value"}) updated = _run( _call_tool( mcp, "update_item", - {"item_id": created["id"], "data": data_str}, + {"item_id": created["id"], "name": "Updated", "detail": detail_str}, ) ) assert updated["name"] == "Updated" + assert updated["detail"] == {"nested": "value"} def test_plain_string_args_not_mangled(self, mcp): - """Non-JSON string arguments (like item_id) must not be altered.""" - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "Test"}})) + """Non-JSON string arguments (like item_id, name) must not be altered.""" + created = _run(_call_tool(mcp, "create_item", {"name": "Test"})) fetched = _run(_call_tool(mcp, "get_item", {"item_id": created["id"]})) assert fetched["id"] == created["id"] + assert fetched["name"] == "Test" - def test_dict_args_still_work(self, mcp): - """Native dict arguments (normal in-process path) must keep working.""" - created = _run(_call_tool(mcp, "create_item", {"data": {"name": "DictWidget"}})) + def test_native_dict_args_still_work(self, mcp): + """Native flat kwargs (normal in-process path) must keep working.""" + created = _run(_call_tool(mcp, "create_item", {"name": "DictWidget"})) assert created["name"] == "DictWidget" +class TestLegacyDataWrapperRejected: + """Regression: the pre-0.5.0 `{data: {...}}` shape is no longer part of the + contract. The canonical call shape is flat kwargs. + + ``create_*`` tools enforce this naturally through JSON Schema validation — + a call with only a ``data`` key is missing every required entity field and + the client raises ``ToolError`` before reaching the server. ``update_*`` + tools accept arbitrary fields by design (partial merge) so they do not + raise on stray keys; consistency there is carried by the published schema + and ``examples``, not by runtime rejection. + """ + + @pytest.fixture + def mcp(self, tmp_path): + manifest_path = _make_manifest( + tmp_path, + [{"name": "item", "plural": "items", "prefix": "it"}], + ) + workspace = tmp_path / "workspace" + workspace.mkdir() + return create_server(manifest_path, root=workspace) + + def test_create_rejects_data_wrapper(self, mcp): + from fastmcp.exceptions import ToolError + + with pytest.raises(ToolError): + _run(_call_tool(mcp, "create_item", {"data": {"name": "Wrapped"}})) + + # =========================================================================== # Seed tool tests # =========================================================================== @@ -1114,7 +1273,7 @@ def test_adds_to_required_when_required_true(self, setup): def test_reload_works_new_entities_get_default(self, setup): mcp, _, _ = setup # Create entity before adding field - created = _run(_call_tool(mcp, "create_widget", {"data": {"name": "Test"}})) + created = _run(_call_tool(mcp, "create_widget", {"name": "Test"})) # Add field with default _run( @@ -1320,16 +1479,14 @@ def test_relationship_tools_registered(self, setup): def test_query_by_relationship_through_mcp(self, setup): mcp = setup - company = _run(_call_tool(mcp, "create_company", {"data": {"name": "Acme"}})) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme"})) _run( _call_tool( mcp, "create_contact", { - "data": { - "name": "Alice", - "relationships": [{"rel": "works_at", "target": company["id"]}], - } + "name": "Alice", + "relationships": [{"rel": "works_at", "target": company["id"]}], }, ) ) @@ -1346,16 +1503,14 @@ def test_query_by_relationship_through_mcp(self, setup): def test_get_related_forward_through_mcp(self, setup): mcp = setup - company = _run(_call_tool(mcp, "create_company", {"data": {"name": "Acme"}})) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme"})) contact = _run( _call_tool( mcp, "create_contact", { - "data": { - "name": "Alice", - "relationships": [{"rel": "works_at", "target": company["id"]}], - } + "name": "Alice", + "relationships": [{"rel": "works_at", "target": company["id"]}], }, ) ) @@ -1372,16 +1527,14 @@ def test_get_related_forward_through_mcp(self, setup): def test_get_related_reverse_through_mcp(self, setup): mcp = setup - company = _run(_call_tool(mcp, "create_company", {"data": {"name": "Acme"}})) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme"})) contact = _run( _call_tool( mcp, "create_contact", { - "data": { - "name": "Alice", - "relationships": [{"rel": "works_at", "target": company["id"]}], - } + "name": "Alice", + "relationships": [{"rel": "works_at", "target": company["id"]}], }, ) ) @@ -1398,16 +1551,14 @@ def test_get_related_reverse_through_mcp(self, setup): def test_get_composite_through_mcp(self, setup): mcp = setup - company = _run(_call_tool(mcp, "create_company", {"data": {"name": "Acme"}})) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme"})) contact = _run( _call_tool( mcp, "create_contact", { - "data": { - "name": "Alice", - "relationships": [{"rel": "works_at", "target": company["id"]}], - } + "name": "Alice", + "relationships": [{"rel": "works_at", "target": company["id"]}], }, ) ) @@ -1425,16 +1576,14 @@ def test_get_composite_through_mcp(self, setup): def test_rebuild_index_through_mcp(self, setup): mcp = setup - company = _run(_call_tool(mcp, "create_company", {"data": {"name": "Acme"}})) + company = _run(_call_tool(mcp, "create_company", {"name": "Acme"})) _run( _call_tool( mcp, "create_contact", { - "data": { - "name": "Alice", - "relationships": [{"rel": "works_at", "target": company["id"]}], - } + "name": "Alice", + "relationships": [{"rel": "works_at", "target": company["id"]}], }, ) ) @@ -1506,9 +1655,7 @@ def test_activity_crud_not_registered_when_disabled(self, mcp_without_activities assert "list_activities" not in tools def test_log_activity_creates_activity_with_relationship(self, mcp_with_activities): - contact = _run( - _call_tool(mcp_with_activities, "create_contact", {"data": {"name": "Alice"}}) - ) + contact = _run(_call_tool(mcp_with_activities, "create_contact", {"name": "Alice"})) activity = _run( _call_tool( mcp_with_activities, @@ -1528,7 +1675,7 @@ def test_log_activity_creates_activity_with_relationship(self, mcp_with_activiti assert any(r["rel"] == "subject" and r["target"] == contact["id"] for r in rels) def test_log_activity_without_detail(self, mcp_with_activities): - contact = _run(_call_tool(mcp_with_activities, "create_contact", {"data": {"name": "Bob"}})) + contact = _run(_call_tool(mcp_with_activities, "create_contact", {"name": "Bob"})) activity = _run( _call_tool( mcp_with_activities, @@ -1540,9 +1687,7 @@ def test_log_activity_without_detail(self, mcp_with_activities): assert activity["detail"] == {} def test_get_activities_returns_activities_for_subject(self, mcp_with_activities): - contact = _run( - _call_tool(mcp_with_activities, "create_contact", {"data": {"name": "Alice"}}) - ) + contact = _run(_call_tool(mcp_with_activities, "create_contact", {"name": "Alice"})) _run( _call_tool( mcp_with_activities, @@ -1570,9 +1715,7 @@ def test_get_activities_returns_activities_for_subject(self, mcp_with_activities assert actions == {"email_sent", "meeting_held"} def test_get_activities_filters_by_action(self, mcp_with_activities): - contact = _run( - _call_tool(mcp_with_activities, "create_contact", {"data": {"name": "Alice"}}) - ) + contact = _run(_call_tool(mcp_with_activities, "create_contact", {"name": "Alice"})) _run( _call_tool( mcp_with_activities, @@ -1617,7 +1760,7 @@ def test_tools_array_filters_listed_tools(self, tmp_path): assert "delete_session" not in listed # Hidden tools are still callable - result = _run(_call_tool(mcp, "create_session", {"data": {"name": "Test"}})) + result = _run(_call_tool(mcp, "create_session", {"name": "Test"})) assert "id" in result def test_tools_array_absent_lists_all(self, tmp_path): @@ -1672,7 +1815,7 @@ def test_empty_tools_array_lists_nothing(self, tmp_path): assert session_tools == set() # But tools are still callable - result = _run(_call_tool(mcp, "create_session", {"data": {"name": "Test"}})) + result = _run(_call_tool(mcp, "create_session", {"name": "Test"})) assert "id" in result def test_graph_traversal_categories(self, tmp_path): diff --git a/lib/python/uv.lock b/lib/python/uv.lock index 262ea39..d546f7e 100644 --- a/lib/python/uv.lock +++ b/lib/python/uv.lock @@ -1179,7 +1179,7 @@ wheels = [ [[package]] name = "upjack" -version = "0.4.2" +version = "0.5.0" source = { editable = "." } dependencies = [ { name = "jsonschema" }, diff --git a/lib/typescript/src/activity.ts b/lib/typescript/src/activity.ts index ec31606..76543d7 100644 --- a/lib/typescript/src/activity.ts +++ b/lib/typescript/src/activity.ts @@ -8,10 +8,10 @@ * Opt-in via "activities": true in the manifest's upjack extension. */ -import { readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import type { EntityDefinition } from "./entity.js"; +import { loadSchema } from "./schema.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); const SCHEMA_PATH = join(__dirname, "schemas", "activity.schema.json"); @@ -23,6 +23,7 @@ export const ACTIVITY_ENTITY_DEF: EntityDefinition = { schema: SCHEMA_PATH, }; +/** Load the built-in activity schema (with base-entity $ref inlined). */ export function getActivitySchema(): Record { - return JSON.parse(readFileSync(SCHEMA_PATH, "utf-8")); + return loadSchema(SCHEMA_PATH); } diff --git a/lib/typescript/src/schema.ts b/lib/typescript/src/schema.ts index fddf07a..8b9a234 100644 --- a/lib/typescript/src/schema.ts +++ b/lib/typescript/src/schema.ts @@ -14,42 +14,109 @@ const addFormats = const __dirname = dirname(fileURLToPath(import.meta.url)); +/** + * Canonical `$id` / `$ref` URL for the bundled base entity schema. App + * schemas reference this via `allOf: [{"$ref": BASE_ENTITY_REF}]` so apps + * can layer their own fields on top of the framework-managed ones. + */ +export const BASE_ENTITY_REF = "https://upjack.dev/schemas/v1/upjack-entity.schema.json"; + +/** + * Non-standard marker attached to the inlined base-entity schema so + * downstream code can identify it without the `$ref` or `$id` (both of + * which cause problems for JSON Schema validators that auto-register + * schemas by `$id`). Consumers should treat any allOf member with this + * field as "the base entity schema, inlined by loadSchema". + */ +export const BASE_ENTITY_MARKER = "x-upjack-base-entity"; + +/** + * Framework-managed base entity fields — the canonical set used across both + * SDKs. Stripped from tool input schemas (auto-managed, not user-controlled) + * and also filtered out of author-supplied examples so the published tool + * schema doesn't instruct LLMs to send them. + */ +export const BASE_ENTITY_FIELDS: ReadonlySet = new Set([ + "id", + "type", + "version", + "created_at", + "updated_at", + "created_by", + "status", + "tags", + "source", + "relationships", +]); + const BASE_SCHEMA_PATH = join(__dirname, "schemas", "upjack-entity.schema.json"); -const BASE_SCHEMA = JSON.parse(readFileSync(BASE_SCHEMA_PATH, "utf-8")); +const BASE_SCHEMA = JSON.parse(readFileSync(BASE_SCHEMA_PATH, "utf-8")) as Record; // @ts-expect-error -- AJV constructor works at runtime despite CJS type mismatch const ajv = new Ajv2020({ allErrors: true, strict: false }); // @ts-expect-error -- ajv-formats works at runtime despite CJS type mismatch addFormats(ajv); -// Register the base schema under its remote URI so $ref resolution works offline -ajv.addSchema(BASE_SCHEMA, "https://upjack.dev/schemas/v1/upjack-entity.schema.json"); - -// Map $ref URIs to resolved schemas for hydration -const REF_MAP: Record> = { - "https://upjack.dev/schemas/v1/upjack-entity.schema.json": BASE_SCHEMA as Record, -}; +// Register the base schema so AJV resolves $ref to it locally (defensive +// guard against schemas that bypass loadSchema). +ajv.addSchema(BASE_SCHEMA, BASE_ENTITY_REF); /** - * Load a JSON Schema from a file path. + * Load a JSON Schema from disk and inline the base-entity `$ref`. * - * @param path - Path to the .schema.json file. - * @returns Parsed JSON Schema object. - * @throws Error if the file doesn't exist or isn't valid JSON. + * Any `allOf: [{"$ref": BASE_ENTITY_REF}]` entry is replaced with the + * bundled base-entity schema inline, so every downstream consumer sees a + * fully self-contained schema. This is the single source of truth for + * $ref resolution — no caller needs to do it again. */ export function loadSchema(path: string): Record { - return JSON.parse(readFileSync(path, "utf-8")); + const schema = JSON.parse(readFileSync(path, "utf-8")); + inlineBaseEntityRef(schema); + return schema; +} + +function inlineBaseEntityRef(node: unknown): void { + if (Array.isArray(node)) { + for (const item of node) inlineBaseEntityRef(item); + return; + } + if (node === null || typeof node !== "object") return; + + const obj = node as Record; + const allOf = obj.allOf; + if (Array.isArray(allOf)) { + for (let i = 0; i < allOf.length; i++) { + const sub = allOf[i]; + if ( + sub && + typeof sub === "object" && + (sub as Record).$ref === BASE_ENTITY_REF + ) { + // Drop $schema and $id — the inlined copy sharing its $id with the + // pre-registered schema in AJV's registry triggers a "resolves to + // more than one schema" error. Our own marker lets downstream code + // recognise this member without those identifiers. + const { + $schema: _s, + $id: _i, + ...inlined + } = structuredClone(BASE_SCHEMA) as Record; + inlined[BASE_ENTITY_MARKER] = true; + allOf[i] = inlined; + } + } + } + for (const value of Object.values(obj)) { + inlineBaseEntityRef(value); + } } /** * Validate entity data against a JSON Schema. * - * Uses JSON Schema draft 2020-12 validation. Resolves $ref to the - * base entity schema via a bundled local copy. - * - * @param data - Entity data to validate. - * @param schema - JSON Schema to validate against. - * @throws Error if validation fails, with details of all errors. + * The AJV registry resolves any remaining `$ref` to the base entity schema + * locally, so validation works even if the caller handed us a schema that + * bypassed `loadSchema`. */ export function validateEntity( data: Record, @@ -68,10 +135,6 @@ export function validateEntity( * Create a composed schema from base entity schema and app-specific schema. * * Uses allOf composition so both base and app constraints apply. - * - * @param baseSchema - The upjack-entity base schema. - * @param appSchema - The app-specific entity schema. - * @returns Composed schema with allOf referencing both. */ export function resolveEntitySchema( baseSchema: Record, @@ -86,8 +149,9 @@ export function resolveEntitySchema( /** * Fill missing fields from schema defaults. * - * Walks `properties` and `allOf` sub-schemas (resolving `$ref` to the - * bundled base entity schema). Does NOT mutate the input data. + * Walks `properties` and `allOf` sub-schemas. Assumes `schema` has been + * loaded via {@link loadSchema} so any base-entity `$ref` has been inlined — + * does not resolve live `$ref` values. */ export function hydrateDefaults( data: Record, @@ -102,14 +166,10 @@ function applyPropertyDefaults( data: Record, schema: Record, ): void { - // Handle allOf — walk each sub-schema const allOf = schema.allOf as Array> | undefined; if (allOf) { for (const sub of allOf) { - const ref = sub.$ref as string | undefined; - if (ref && ref in REF_MAP) { - applyPropertyDefaults(data, REF_MAP[ref]); - } else { + if (sub && typeof sub === "object") { applyPropertyDefaults(data, sub); } } @@ -141,7 +201,6 @@ export function validateSchemaChange( const oldRequired = new Set((oldSchema.required ?? []) as string[]); const newRequired = new Set((newSchema.required ?? []) as string[]); - // Newly required without default for (const field of [...newRequired].sort()) { if (oldRequired.has(field)) continue; const prop = newProps[field]; @@ -154,7 +213,6 @@ export function validateSchemaChange( } } - // Type change and enum narrowing on shared fields const sharedFields = Object.keys(oldProps) .filter((k) => k in newProps) .sort(); @@ -185,7 +243,6 @@ export function validateSchemaChange( } } - // Field removed for (const field of Object.keys(oldProps).sort()) { if (!(field in newProps)) { diagnostics.push({ @@ -202,23 +259,15 @@ export function validateSchemaChange( /** * Build an output schema for a single-entity tool response. * - * Strips JSON Schema meta keywords and ensures `type: "object"`. + * Expects `schema` to be already self-contained (loaded via {@link loadSchema}). + * Strips JSON Schema meta keywords that don't belong in a tool output schema. + * MCP requires `type: "object"` on every outputSchema. */ export function buildEntityOutputSchema(schema: Record): Record { const { $schema: _, $id: __, ...result } = structuredClone(schema); if (!("type" in result)) { result.type = "object"; } - // Resolve $ref in allOf to prevent downstream AJV resolution failures - if (Array.isArray(result.allOf)) { - result.allOf = (result.allOf as Array>).map((sub) => { - const ref = sub.$ref as string | undefined; - if (ref && ref in REF_MAP) { - return structuredClone(REF_MAP[ref]); - } - return sub; - }); - } return result; } diff --git a/lib/typescript/src/server.ts b/lib/typescript/src/server.ts index fc55264..566c2a3 100644 --- a/lib/typescript/src/server.ts +++ b/lib/typescript/src/server.ts @@ -19,25 +19,19 @@ import { UpjackApp } from "./app.js"; import type { UpjackManifestExtension } from "./app.js"; import { rebuildIndex } from "./relations.js"; import { + BASE_ENTITY_FIELDS, + BASE_ENTITY_MARKER, + BASE_ENTITY_REF, buildEntityOutputSchema, buildListOutputSchema, loadSchema, validateSchemaChange, } from "./schema.js"; -// Base entity fields auto-managed by the framework — stripped from tool input schemas -const BASE_ENTITY_KEYS = new Set([ - "id", - "type", - "version", - "created_at", - "updated_at", - "created_by", - "status", - "tags", - "source", - "relationships", -]); +// Re-export under the local alias used throughout this module. Importing +// the single canonical set from schema.ts keeps the Python and TypeScript +// SDKs in lockstep — a parity test ensures the two sets stay aligned. +const BASE_ENTITY_KEYS = BASE_ENTITY_FIELDS; // --------------------------------------------------------------------------- // Schema preparation @@ -47,13 +41,42 @@ interface JsonSchema { type?: string; properties?: Record; required?: string[]; + allOf?: Array>; + examples?: unknown[]; $schema?: string; $id?: string; [key: string]: unknown; } +function isBaseEntitySchema(node: unknown): boolean { + if (!node || typeof node !== "object") return false; + const obj = node as Record; + // Match either the raw $ref (schemas that bypassed loadSchema) or the + // marker we attach during inlining (the common path). + return obj.$ref === BASE_ENTITY_REF || obj[BASE_ENTITY_MARKER] === true; +} + +/** + * Project an app entity schema onto an MCP tool input schema. + * + * Tool inputs carry only user-controlled fields — framework-managed base + * fields (id, type, timestamps, status, tags, source, relationships) are + * excluded. Assumes `schema` has been loaded via `loadSchema` so any + * base-entity `$ref` has already been inlined. + */ function prepareEntitySchema(schema: JsonSchema, opts?: { forUpdate?: boolean }): JsonSchema { - const { $schema: _, $id: __, ...result } = structuredClone(schema); + const { $schema: _s, $id: _i, examples: _e, ...initial } = structuredClone(schema); + let result = initial; + + if (Array.isArray(result.allOf)) { + const filteredAllOf = result.allOf.filter((sub) => !isBaseEntitySchema(sub)); + if (filteredAllOf.length === 0) { + const { allOf: _dropped, ...rest } = result; + result = rest; + } else { + result.allOf = filteredAllOf; + } + } if (result.properties) { result.properties = Object.fromEntries( @@ -67,12 +90,12 @@ function prepareEntitySchema(schema: JsonSchema, opts?: { forUpdate?: boolean }) } if (result.required) { - const filtered = result.required.filter((r) => !BASE_ENTITY_KEYS.has(r)); - if (filtered.length === 0) { + const filteredRequired = result.required.filter((r) => !BASE_ENTITY_KEYS.has(r)); + if (filteredRequired.length === 0) { const { required: _req, ...rest } = result; return rest; } - result.required = filtered; + result.required = filteredRequired; } return result; @@ -143,11 +166,50 @@ function buildEntityTools( const plural = entityDef.plural ?? `${name}s`; const prefix = entityDef.prefix; const idHint = `IDs start with ${prefix}_`; + const idParam = `${name}_id`; + const idPlaceholder = `${prefix}_01HXXX`; + + // Base schemas for create/update — flat, no `data` wrapper + const createSchema: Record = schema + ? (prepareEntitySchema(schema as JsonSchema) as Record) + : { type: "object" }; + if (!("type" in createSchema)) createSchema.type = "object"; - const dataSchema = schema ? prepareEntitySchema(schema as JsonSchema) : { type: "object" }; - const updateDataSchema = schema - ? prepareEntitySchema(schema as JsonSchema, { forUpdate: true }) + const updateFieldsSchema = schema + ? (prepareEntitySchema(schema as JsonSchema, { forUpdate: true }) as Record) : { type: "object" }; + const updateExtraProps = (updateFieldsSchema.properties ?? {}) as Record; + + const updateSchema: Record = { + type: "object", + properties: { + [idParam]: { type: "string", description: `${name} ID (${prefix}_...)` }, + ...updateExtraProps, + }, + required: [idParam], + }; + + // Author-supplied examples: pass-through, with base entity fields stripped + // (those are auto-managed and shouldn't appear in tool examples). + const rawExamples = Array.isArray((schema as JsonSchema | undefined)?.examples) + ? ((schema as JsonSchema).examples as unknown[]) + : []; + const authorExamples: Array> = []; + for (const ex of rawExamples) { + if (ex && typeof ex === "object" && !Array.isArray(ex)) { + const filtered = Object.fromEntries( + Object.entries(ex as Record).filter(([k]) => !BASE_ENTITY_KEYS.has(k)), + ); + authorExamples.push(filtered); + } + } + if (authorExamples.length > 0) { + createSchema.examples = structuredClone(authorExamples); + updateSchema.examples = authorExamples.map((ex) => ({ + [idParam]: idPlaceholder, + ...ex, + })); + } const entityOut = schema ? buildEntityOutputSchema(schema) : undefined; const listOut = schema ? buildListOutputSchema(schema) : undefined; @@ -156,11 +218,7 @@ function buildEntityTools( { name: `create_${name}`, description: `Create a new ${name}. ${idHint}.`, - inputSchema: { - type: "object", - properties: { data: dataSchema }, - required: ["data"], - }, + inputSchema: createSchema, ...(entityOut ? { outputSchema: entityOut } : {}), }, { @@ -169,23 +227,17 @@ function buildEntityTools( inputSchema: { type: "object", properties: { - entity_id: { type: "string", description: `${name} ID (${prefix}_...)` }, + [idParam]: { type: "string", description: `${name} ID (${prefix}_...)` }, }, - required: ["entity_id"], + required: [idParam], + examples: [{ [idParam]: idPlaceholder }], }, ...(entityOut ? { outputSchema: entityOut } : {}), }, { name: `update_${name}`, - description: `Update a ${name} by ID. Merges fields by default. ${idHint}.`, - inputSchema: { - type: "object", - properties: { - entity_id: { type: "string", description: `${name} ID (${prefix}_...)` }, - data: updateDataSchema, - }, - required: ["entity_id", "data"], - }, + description: `Update a ${name} by ID. Merges fields by default — pass any subset of fields to change. Unknown fields are merged onto the entity as-is (the schema does not enforce additionalProperties=false). ${idHint}.`, + inputSchema: updateSchema, ...(entityOut ? { outputSchema: entityOut } : {}), }, { @@ -222,25 +274,31 @@ function buildEntityTools( inputSchema: { type: "object", properties: { - entity_id: { type: "string", description: `${name} ID` }, + [idParam]: { type: "string", description: `${name} ID (${prefix}_...)` }, hard: { type: "boolean", default: false, description: "Hard delete" }, }, - required: ["entity_id"], + required: [idParam], + examples: [{ [idParam]: idPlaceholder }], }, ...(entityOut ? { outputSchema: entityOut } : {}), }, ]; const handlers: Record = { - [`create_${name}`]: (args) => - app.createEntity(name, (args.data ?? {}) as Record), - [`get_${name}`]: (args) => app.getEntity(name, args.entity_id as string), - [`update_${name}`]: (args) => - app.updateEntity( - name, - args.entity_id as string, - (args.data ?? {}) as Record, - ), + [`create_${name}`]: (args) => app.createEntity(name, args), + [`get_${name}`]: (args) => app.getEntity(name, args[idParam] as string), + [`update_${name}`]: (args) => { + // NB: we strip the id param from the payload before merging. If an app + // ever declares a top-level property literally named `{entity_name}_id` + // (e.g. `user_id` on a `user` entity), it becomes unreachable via the + // update tool. Avoid that collision in your schema. + const id = args[idParam] as string; + const rest: Record = {}; + for (const [k, v] of Object.entries(args)) { + if (k !== idParam) rest[k] = v; + } + return app.updateEntity(name, id, rest); + }, [`list_${plural}`]: (args) => wrapList( app.listEntities(name, (args.status as string) ?? "active", (args.limit as number) ?? 50), @@ -255,7 +313,7 @@ function buildEntityTools( }), ), [`delete_${name}`]: (args) => - app.deleteEntity(name, args.entity_id as string, (args.hard as boolean) ?? false), + app.deleteEntity(name, args[idParam] as string, (args.hard as boolean) ?? false), }; return { definitions, handlers }; @@ -440,18 +498,9 @@ function buildActivityTools(app: UpjackApp): { // --------------------------------------------------------------------------- const FIELD_NAME_RE = /^[a-z][a-z0-9_]*$/; -const BASE_ENTITY_FIELD_NAMES = new Set([ - "id", - "type", - "version", - "created_at", - "updated_at", - "created_by", - "status", - "tags", - "source", - "relationships", -]); +// Reserved field names for add_field — same canonical set as the tool-input +// strip list. +const BASE_ENTITY_FIELD_NAMES = BASE_ENTITY_FIELDS; // Fields stripped from seed data before create — matches Python behavior. // Preserves relationships and tags so seed data can set up a connected graph. diff --git a/lib/typescript/tests/activity.test.ts b/lib/typescript/tests/activity.test.ts index 777f6dd..9666149 100644 --- a/lib/typescript/tests/activity.test.ts +++ b/lib/typescript/tests/activity.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, mkdirSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { beforeEach, describe, expect, it } from "vitest"; diff --git a/lib/typescript/tests/e2e.test.ts b/lib/typescript/tests/e2e.test.ts index e3a0c67..efb30d1 100644 --- a/lib/typescript/tests/e2e.test.ts +++ b/lib/typescript/tests/e2e.test.ts @@ -522,7 +522,7 @@ describe("CRM Server E2E", () => { it("creates contact through tool", async () => { const result = await client.callTool({ name: "create_contact", - arguments: { data: { first_name: "Sarah", last_name: "Chen" } }, + arguments: { first_name: "Sarah", last_name: "Chen" }, }); const contact = callToolJson(result); expect((contact.id as string).startsWith("ct_")).toBe(true); @@ -614,7 +614,7 @@ describe("Todo Server E2E", () => { it("creates task through tool", async () => { const result = await client.callTool({ name: "create_task", - arguments: { data: { title: "Buy groceries" } }, + arguments: { title: "Buy groceries" }, }); const task = callToolJson(result); expect((task.id as string).startsWith("tsk_")).toBe(true); @@ -625,7 +625,7 @@ describe("Todo Server E2E", () => { // Create const createResult = await client.callTool({ name: "create_task", - arguments: { data: { title: "Test task", priority: "high" } }, + arguments: { title: "Test task", priority: "high" }, }); const created = callToolJson(createResult); const id = created.id as string; @@ -634,7 +634,7 @@ describe("Todo Server E2E", () => { // Get const getResult = await client.callTool({ name: "get_task", - arguments: { entity_id: id }, + arguments: { task_id: id }, }); const fetched = callToolJson(getResult); expect(fetched.title).toBe("Test task"); @@ -642,7 +642,7 @@ describe("Todo Server E2E", () => { // Update const updateResult = await client.callTool({ name: "update_task", - arguments: { entity_id: id, data: { priority: "critical" } }, + arguments: { task_id: id, priority: "critical" }, }); const updated = callToolJson(updateResult); expect(updated.priority).toBe("critical"); @@ -669,7 +669,7 @@ describe("Todo Server E2E", () => { // Delete const deleteResult = await client.callTool({ name: "delete_task", - arguments: { entity_id: id }, + arguments: { task_id: id }, }); const deleted = callToolJson(deleteResult); expect(deleted.status).toBe("deleted"); diff --git a/lib/typescript/tests/schema.test.ts b/lib/typescript/tests/schema.test.ts index 25329e0..ef77fee 100644 --- a/lib/typescript/tests/schema.test.ts +++ b/lib/typescript/tests/schema.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { + BASE_ENTITY_FIELDS, buildEntityOutputSchema, buildListOutputSchema, hydrateDefaults, @@ -13,6 +14,29 @@ import { validateSchemaChange, } from "../src/schema.js"; +describe("SDK parity", () => { + // Pin the cross-SDK contract so Python and TypeScript stay aligned. The + // Python SDK has a mirror test (see tests/test_schema.py TestSdkParity) + // asserting the same canonical set. If you change one, change the other — + // and update CHANGELOG to note the tool-schema delta. + it("BASE_ENTITY_FIELDS is the canonical set (matches Python)", () => { + expect(new Set(BASE_ENTITY_FIELDS)).toEqual( + new Set([ + "id", + "type", + "version", + "created_at", + "updated_at", + "created_by", + "status", + "tags", + "source", + "relationships", + ]), + ); + }); +}); + describe("loadSchema", () => { it("loads a valid schema file", () => { const tmp = mkdtempSync(join(tmpdir(), "schema-")); @@ -136,18 +160,27 @@ describe("hydrateDefaults", () => { expect(data).toEqual({ name: "test" }); }); - it("handles allOf with $ref to base entity schema", () => { - const schema = { - allOf: [ - { $ref: "https://upjack.dev/schemas/v1/upjack-entity.schema.json" }, - { - type: "object", - properties: { - priority: { type: "string", default: "medium" }, + it("handles inlined allOf base entity schema from loadSchema", () => { + // Schemas loaded via loadSchema() carry the base entity inlined under + // allOf — hydrateDefaults walks those inlined members to pull defaults + // (status, tags, relationships, etc.) alongside app-level defaults. + const tmpDir = mkdtempSync(join(tmpdir(), "upjack-schema-test-")); + const schemaPath = join(tmpDir, "contact.schema.json"); + writeFileSync( + schemaPath, + JSON.stringify({ + allOf: [ + { $ref: "https://upjack.dev/schemas/v1/upjack-entity.schema.json" }, + { + type: "object", + properties: { + priority: { type: "string", default: "medium" }, + }, }, - }, - ], - }; + ], + }), + ); + const schema = loadSchema(schemaPath); const data = { name: "test" }; const result = hydrateDefaults(data, schema); // Base schema has defaults for created_by, status, tags, relationships diff --git a/lib/typescript/tests/server.test.ts b/lib/typescript/tests/server.test.ts index cee442e..a6c5022 100644 --- a/lib/typescript/tests/server.test.ts +++ b/lib/typescript/tests/server.test.ts @@ -298,7 +298,7 @@ describe("createServer", () => { }); describe("tool input schemas", () => { - it("create tool exposes entity schema with full field structure", async () => { + it("create tool exposes entity fields at top level (no data wrapper)", async () => { const entitySchema = { $schema: "https://json-schema.org/draft/2020-12/schema", type: "object", @@ -317,7 +317,6 @@ describe("tool input schemas", () => { required: ["name"], }; - // makeManifest writes a default schema; overwrite it after const manifestPath = makeManifest(tmpDir, [ { name: "campaign", plural: "campaigns", prefix: "cp" }, ]); @@ -326,33 +325,28 @@ describe("tool input schemas", () => { const tools = await client.listTools(); const createTool = tools.tools.find((t) => t.name === "create_campaign"); expect(createTool).toBeDefined(); - const dataSchema = ( - createTool?.inputSchema.properties as Record> - ).data; - - // App fields present with full metadata - expect( - (dataSchema.properties as Record>).name.description, - ).toBe("Campaign name"); - expect((dataSchema.properties as Record>).score.minimum).toBe( - 0, - ); + const inputSchema = createTool?.inputSchema; + const props = inputSchema?.properties as Record>; + + // No `data` wrapper — fields are flat at the top level + expect(props).not.toHaveProperty("data"); + expect(props.name.description).toBe("Campaign name"); + expect(props.score.minimum).toBe(0); // Nested structure preserved - const drivers = (dataSchema.properties as Record>) - .emotional_drivers; + const drivers = props.emotional_drivers; expect((drivers.properties as Record>).fear).toBeDefined(); // Base fields stripped - expect(dataSchema.properties).not.toHaveProperty("id"); - expect(dataSchema.properties).not.toHaveProperty("type"); - // $schema stripped - expect(dataSchema).not.toHaveProperty("$schema"); + expect(props).not.toHaveProperty("id"); + expect(props).not.toHaveProperty("type"); + // $schema stripped at top level + expect(inputSchema).not.toHaveProperty("$schema"); // Required preserved (minus base fields) - expect(dataSchema.required).toEqual(["name"]); + expect(inputSchema?.required).toEqual(["name"]); await client.close(); }); - it("update tool has no required in data (partial merge)", async () => { + it("update tool has flat fields and entity id, no data wrapper", async () => { const entitySchema = { $schema: "https://json-schema.org/draft/2020-12/schema", type: "object", @@ -370,16 +364,15 @@ describe("tool input schemas", () => { const updateTool = tools.tools.find((t) => t.name === "update_item"); expect(updateTool).toBeDefined(); const inputSchema = updateTool?.inputSchema; + const props = inputSchema?.properties as Record>; - // Top-level requires entity_id and data - expect(inputSchema.properties).toHaveProperty("entity_id"); - expect(new Set(inputSchema.required as string[])).toEqual(new Set(["entity_id", "data"])); - // Data schema has no required (partial update) - const dataSchema = (inputSchema.properties as Record>).data; - expect(dataSchema).not.toHaveProperty("required"); - // But fields are still described - expect(dataSchema.properties).toHaveProperty("name"); - expect(dataSchema.properties).toHaveProperty("score"); + // Flat: item_id + fields at top level, no `data` wrapper + expect(props).not.toHaveProperty("data"); + expect(props).toHaveProperty("item_id"); + expect(props).toHaveProperty("name"); + expect(props).toHaveProperty("score"); + // Only the id is required (partial-merge semantics for everything else) + expect(inputSchema?.required).toEqual(["item_id"]); await client.close(); }); @@ -400,7 +393,7 @@ describe("tool CRUD", () => { it("create + get roundtrip", async () => { const createResult = await client.callTool({ name: "create_item", - arguments: { data: { name: "Widget" } }, + arguments: { name: "Widget" }, }); const created = JSON.parse((createResult.content as Array<{ text: string }>)[0].text); expect(created.id.startsWith("it_")).toBe(true); @@ -408,7 +401,7 @@ describe("tool CRUD", () => { const getResult = await client.callTool({ name: "get_item", - arguments: { entity_id: created.id }, + arguments: { item_id: created.id }, }); const fetched = JSON.parse((getResult.content as Array<{ text: string }>)[0].text); expect(fetched.id).toBe(created.id); @@ -417,13 +410,13 @@ describe("tool CRUD", () => { it("update merges fields", async () => { const createResult = await client.callTool({ name: "create_item", - arguments: { data: { name: "Old" } }, + arguments: { name: "Old" }, }); const created = JSON.parse((createResult.content as Array<{ text: string }>)[0].text); const updateResult = await client.callTool({ name: "update_item", - arguments: { entity_id: created.id, data: { name: "New", extra: "field" } }, + arguments: { item_id: created.id, name: "New", extra: "field" }, }); const updated = JSON.parse((updateResult.content as Array<{ text: string }>)[0].text); expect(updated.name).toBe("New"); @@ -431,8 +424,8 @@ describe("tool CRUD", () => { }); it("list returns created entities in envelope", async () => { - await client.callTool({ name: "create_item", arguments: { data: { name: "A" } } }); - await client.callTool({ name: "create_item", arguments: { data: { name: "B" } } }); + await client.callTool({ name: "create_item", arguments: { name: "A" } }); + await client.callTool({ name: "create_item", arguments: { name: "B" } }); const listResult = await client.callTool({ name: "list_items", arguments: {} }); const result = JSON.parse((listResult.content as Array<{ text: string }>)[0].text); @@ -441,8 +434,8 @@ describe("tool CRUD", () => { }); it("search finds by text in envelope", async () => { - await client.callTool({ name: "create_item", arguments: { data: { name: "Alpha" } } }); - await client.callTool({ name: "create_item", arguments: { data: { name: "Beta" } } }); + await client.callTool({ name: "create_item", arguments: { name: "Alpha" } }); + await client.callTool({ name: "create_item", arguments: { name: "Beta" } }); const searchResult = await client.callTool({ name: "search_items", @@ -456,13 +449,13 @@ describe("tool CRUD", () => { it("soft delete", async () => { const createResult = await client.callTool({ name: "create_item", - arguments: { data: { name: "Doomed" } }, + arguments: { name: "Doomed" }, }); const created = JSON.parse((createResult.content as Array<{ text: string }>)[0].text); const deleteResult = await client.callTool({ name: "delete_item", - arguments: { entity_id: created.id }, + arguments: { item_id: created.id }, }); const deleted = JSON.parse((deleteResult.content as Array<{ text: string }>)[0].text); expect(deleted.status).toBe("deleted"); @@ -471,6 +464,17 @@ describe("tool CRUD", () => { const result = JSON.parse((listResult.content as Array<{ text: string }>)[0].text); expect(result.entities).toHaveLength(0); }); + + it("legacy {data: {...}} shape is rejected", async () => { + // 0.5.0 removed the data wrapper. A call using the old shape is missing + // the required `name` field at the top level, so the schema validator + // rejects it rather than silently succeeding. + const result = await client.callTool({ + name: "create_item", + arguments: { data: { name: "Wrapped" } }, + }); + expect(result.isError).toBeTruthy(); + }); }); describe("JSON string deserialization", () => { @@ -488,35 +492,41 @@ describe("JSON string deserialization", () => { await client.close(); }); - it("create works when data is a JSON string", async () => { + it("create works when array arg arrives as a JSON string", async () => { + // Over stdio transport, nested arrays/objects can arrive as JSON-serialized + // strings. The server parses these transparently. + const relsStr = JSON.stringify([{ rel: "belongs_to", target: "it_abc" }]); const result = await client.callTool({ name: "create_item", - arguments: { data: JSON.stringify({ name: "StringWidget" }) }, + arguments: { name: "StringWidget", relationships: relsStr }, }); const created = JSON.parse((result.content as Array<{ text: string }>)[0].text); expect(created.name).toBe("StringWidget"); - expect(created.id.startsWith("it_")).toBe(true); + expect(Array.isArray(created.relationships)).toBe(true); + expect(created.relationships[0].rel).toBe("belongs_to"); }); - it("update works when data is a JSON string", async () => { + it("update works when object arg arrives as a JSON string", async () => { const createResult = await client.callTool({ name: "create_item", - arguments: { data: { name: "Original" } }, + arguments: { name: "Original" }, }); const created = JSON.parse((createResult.content as Array<{ text: string }>)[0].text); + const detailStr = JSON.stringify({ nested: "value" }); const updateResult = await client.callTool({ name: "update_item", - arguments: { entity_id: created.id, data: JSON.stringify({ name: "Updated" }) }, + arguments: { item_id: created.id, name: "Updated", detail: detailStr }, }); const updated = JSON.parse((updateResult.content as Array<{ text: string }>)[0].text); expect(updated.name).toBe("Updated"); + expect(updated.detail).toEqual({ nested: "value" }); }); it("search works when filter is a JSON string", async () => { await client.callTool({ name: "create_item", - arguments: { data: { name: "Findme" } }, + arguments: { name: "Findme" }, }); const searchResult = await client.callTool({ @@ -531,13 +541,13 @@ describe("JSON string deserialization", () => { it("plain string args are not mangled", async () => { const createResult = await client.callTool({ name: "create_item", - arguments: { data: { name: "Test" } }, + arguments: { name: "Test" }, }); const created = JSON.parse((createResult.content as Array<{ text: string }>)[0].text); const getResult = await client.callTool({ name: "get_item", - arguments: { entity_id: created.id }, + arguments: { item_id: created.id }, }); const fetched = JSON.parse((getResult.content as Array<{ text: string }>)[0].text); expect(fetched.id).toBe(created.id); @@ -546,7 +556,7 @@ describe("JSON string deserialization", () => { it("dict args still work (normal in-process path)", async () => { const result = await client.callTool({ name: "create_item", - arguments: { data: { name: "DictWidget" } }, + arguments: { name: "DictWidget" }, }); const created = JSON.parse((result.content as Array<{ text: string }>)[0].text); expect(created.name).toBe("DictWidget"); @@ -637,7 +647,7 @@ describe("tool listing filter", () => { // Hidden tools are still callable const result = await client.callTool({ name: "create_session", - arguments: { data: { name: "Test" } }, + arguments: { name: "Test" }, }); expect(result.isError).toBeFalsy(); @@ -695,7 +705,7 @@ describe("tool listing filter", () => { // But tools are still callable const result = await client.callTool({ name: "create_session", - arguments: { data: { name: "Test" } }, + arguments: { name: "Test" }, }); expect(result.isError).toBeFalsy(); @@ -749,7 +759,7 @@ describe("add_field tool", () => { // Create an entity — the new field should be accepted const createResult = await client.callTool({ name: "create_widget", - arguments: { data: { name: "Test", color: "red" } }, + arguments: { name: "Test", color: "red" }, }); const created = JSON.parse((createResult.content as Array<{ text: string }>)[0].text); expect(created.color).toBe("red");