diff --git a/packages/ai/src/ai/modules/mcp/doc.md b/packages/ai/src/ai/modules/mcp/doc.md index 979c3ce57..54ead761d 100644 --- a/packages/ai/src/ai/modules/mcp/doc.md +++ b/packages/ai/src/ai/modules/mcp/doc.md @@ -11,7 +11,7 @@ like every other module and mounted at: /mcp ``` -This module exposes a static, 27-tool RocketRide authoring/execution surface served +This module exposes a static, 29-tool RocketRide authoring/execution surface served over HTTP from inside the running engine process — no separate process or transport bridge required. It supersedes the earlier 2-tool port, which exposed a dynamic per-pipeline `{filepath}` tool plus a `RocketRide_Document_Processor` convenience @@ -104,7 +104,7 @@ on every `CacheableResult` this module returns: Config key `mcp_dev_no_auth` (bool, in the module `config` dict passed to `initModule`) is the config-driven equivalent of `MCP_DEV_NO_AUTH=1`; either one enables the bypass. -## The 27 tools +## The 29 tools Dispatch is registry-based: `tooling.ToolRegistry` holds `{name -> (description, inputSchema, handler)}`; `tools/__init__.register_all(registry)` populates one shared @@ -120,18 +120,20 @@ All tools are static and typed (fixed name + JSON Schema) — there is no dynami per-pipeline tool generation and no `filepath`-shaped catch-all tool of the kind the legacy 2-tool port used. -The 27 tools are organized into 8 groups (plus 2 resources), matching +The 29 tools are organized into 8 groups (plus 2 resources), matching `claude/tasks/http-mcp-tools-port/final-tool-surface.md` minus the Query group (see History: the 3 convenience query tools were removed pending their cloud DB backend), plus the Run log (DVR) group and `list_integrations` added below. -**Introspection (5)** — `tools/introspection.py` plus `tools/integrations.py`, read-only/static-analysis, no task tokens: +**Introspection (7)** — `tools/introspection.py` plus `tools/scaffold.py` and `tools/integrations.py`, read-only/static-analysis, no task tokens: | Tool | Purpose | Key args | | --- | --- | --- | | `list_components` | List pipeline components ready to use *now* — zero-config components plus integrations whose credentials are configured. Configured entries carry a `wiring` block of `${VAR}` placeholders; a `note` counts integrations omitted for needing setup. Call `list_integrations` for those. | none | | `describe_component` | Full metadata/config schema for one component; catalog nodes also get a `credentials` block (same readiness vocabulary as `list_integrations`). | `name` (required) | -| `validate_pipeline` | Validate a pipeline against the engine's own rules (engine-authoritative, zero client-side drift). | `pipeline` | +| `resolve_config` | What a component config resolves to at load, after the engine applies profile and default merging. Reports keys the resolver discarded, which a schema cannot express. | `provider` (required), `config` | +| `scaffold_node` | Emit a local node skeleton that loads on the first try, with the manifest keys and file layout the engine requires. Returns files to write. | `name` (required), `lane_in`, `lane_out`, `class_type` | +| `validate_pipeline` | Validate a pipeline against the engine's own rules (engine-authoritative, zero client-side drift), plus a check that every component names a provider the engine has a service for, which the engine's pipeline path does not cover. | `pipeline` | | `describe_pipeline` | Statically describe a pipeline's source and components (id, provider, title, classType, inputs); synthesized client-side, no backing SDK method. | `pipeline` | | `list_integrations` | Credential readiness for catalog integrations this engine has a matching node for. Bare call: terse per-integration rows (`name`/`title`/`status`/`missing_count`). With `name`: full field detail, `missing`, `candidates`, the caller's own variable names (`caller_variables`), and either `setup` (not configured) or `wiring` (configured). | `name` (optional) | @@ -356,7 +358,7 @@ prompt templates from the earlier port were removed along with their tests. ## The `EngineClient` seam `engine.py` defines one `Protocol`, `EngineClient`, with the methods needed -by the 27-tool surface (task lifecycle, services/validation, store/templates/store +by the 29-tool surface (task lifecycle, services/validation, store/templates/store metadata/signed URLs, full deployment lifecycle, `rrext_log` chapters/read/traces/ trace — see the `Protocol` definition in `engine.py` for exact signatures). All tool/resource code depends only on this interface — never on a concrete client — so diff --git a/packages/ai/src/ai/modules/mcp/engine.py b/packages/ai/src/ai/modules/mcp/engine.py index 4a50112cc..f6e9b00c4 100644 --- a/packages/ai/src/ai/modules/mcp/engine.py +++ b/packages/ai/src/ai/modules/mcp/engine.py @@ -40,6 +40,7 @@ async def send( async def get_services(self) -> Dict[str, Any]: ... async def get_service(self, name: str) -> Optional[Dict[str, Any]]: ... async def validate(self, pipeline: dict, source: Optional[str] = None) -> Dict[str, Any]: ... + async def resolve_config(self, provider: str, config: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: ... async def use(self, **kwargs: Any) -> Dict[str, Any]: ... async def terminate(self, token: str) -> None: ... async def send_files(self, files: List[Any], token: str) -> Any: ... @@ -201,6 +202,9 @@ async def get_service(self, name: str) -> Optional[Dict[str, Any]]: async def validate(self, pipeline: dict, source: Optional[str] = None) -> Dict[str, Any]: return await self._guarded(lambda: self._client.validate(pipeline, source=source)) + async def resolve_config(self, provider: str, config: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + return await self._guarded(lambda: self._client.resolve_config(provider, config)) + async def use(self, **kwargs: Any) -> Dict[str, Any]: return await self._guarded(lambda: self._client.use(**kwargs)) diff --git a/packages/ai/src/ai/modules/mcp/tools/__init__.py b/packages/ai/src/ai/modules/mcp/tools/__init__.py index 955791330..38bb49c6a 100644 --- a/packages/ai/src/ai/modules/mcp/tools/__init__.py +++ b/packages/ai/src/ai/modules/mcp/tools/__init__.py @@ -13,6 +13,7 @@ from . import integrations from . import introspection from . import logs +from . import scaffold from . import visibility @@ -27,7 +28,8 @@ def register_all(registry: ToolRegistry) -> None: `save_template`, `load_template`, `deploy_add`, `deploy_list`, `deploy_status`, `deploy_remove`, `deploy_update`), the visibility tools (`monitor`, `list_running_pipelines`), the DVR run-log tools - (`log_chapters`, `log_read`, `log_traces`, `log_trace`), and the + (`log_chapters`, `log_read`, `log_traces`, `log_trace`), the node + scaffolding tool (`scaffold_node`), and the integration-discovery tool (`list_integrations`) -- registered last so it always trails the surface it discovers. """ @@ -36,4 +38,5 @@ def register_all(registry: ToolRegistry) -> None: capability.register(registry) visibility.register(registry) logs.register(registry) + scaffold.register(registry) integrations.register(registry) diff --git a/packages/ai/src/ai/modules/mcp/tools/introspection.py b/packages/ai/src/ai/modules/mcp/tools/introspection.py index d872b5140..210359104 100644 --- a/packages/ai/src/ai/modules/mcp/tools/introspection.py +++ b/packages/ai/src/ai/modules/mcp/tools/introspection.py @@ -100,6 +100,41 @@ async def _describe_component(client, tasks, args: Dict[str, Any]) -> dict: return result +async def _unknown_provider_errors(client, pipeline: Dict[str, Any]): + """Report components naming a provider the engine has no service for. + + The engine validates a whole pipeline structurally. Its provider lookup lives + in the single-component path (`validate_pipeline.cpp`), which the pipeline path + never reaches, so a typed provider name validates clean and fails at run time. + The names come from the engine's own catalog, so this adds no client-side rule. + + Args: + client: The engine client. + pipeline: The pipeline body, already unwrapped from its envelope. + + Returns: + A ``(errors, engine_error)`` pair; ``engine_error`` is set only when the + catalog could not be read. + """ + components = [c for c in (pipeline.get('components') or []) if isinstance(c, dict)] + if not components: + return [], None + + services, err = await engine_call(client.get_services(), 'validate_pipeline') + if err: + return [], err + + catalog = (services or {}).get('services') or {} + if not catalog: + return [], None + + return [ + {'component': c.get('id'), 'message': f'unknown provider {c.get("provider")!r}'} + for c in components + if c.get('provider') and c['provider'] not in catalog + ], None + + async def _validate_pipeline(client, tasks, args: Dict[str, Any]) -> dict: pipeline = load_pipeline(args) # raises ValueError -> normalized by the dispatch layer # The engine requires the {'pipeline': {...}} envelope and treats a missing @@ -113,11 +148,45 @@ async def _validate_pipeline(client, tasks, args: Dict[str, Any]) -> dict: if err: return err result = validated or {} - errors = result.get('errors') or [] + errors = list(result.get('errors') or []) warnings = result.get('warnings') or [] + + unknown, err = await _unknown_provider_errors(client, payload['pipeline']) + if err: + return err + errors.extend(unknown) return {'ok': not errors, 'errors': errors, 'warnings': warnings} +async def _resolve_config(client, tasks, args: Dict[str, Any]) -> dict: + provider = args.get('provider') + if not provider: + return _bad('provider is required', 'pick a provider from list_components') + + # Default only an absent config: `or {}` would coerce [] and skip this check. + config = args.get('config') + if config is None: + config = {} + if not isinstance(config, dict): + return _bad('config must be an object', "pass the component's config block, or omit it for defaults") + + resolved, err = await engine_call(client.resolve_config(provider, config), 'resolve_config') + if err: + return err + + result = resolved or {} + # Surface the discard rather than leaving it to be inferred from an absence: + # a key written beside 'profile' instead of inside it never reaches the node. + dropped = result.get('dropped') or [] + out = {'ok': True, **result} + if dropped: + out['hint'] = ( + f'{len(dropped)} config key(s) were discarded because a profile is set: {", ".join(dropped)}. ' + f'Move them inside the "{result.get("profile")}" object to take effect.' + ) + return out + + async def _describe_pipeline(client, tasks, args: Dict[str, Any]) -> dict: pipeline = load_pipeline(args) # raises ValueError -> normalized by the dispatch layer @@ -161,7 +230,7 @@ async def _describe_pipeline(client, tasks, args: Dict[str, Any]) -> dict: def register(registry: ToolRegistry) -> None: - """Register the 4 authoring/introspection tools against ``registry``.""" + """Register the authoring/introspection tools against ``registry``.""" registry.register( 'list_components', 'List RocketRide components ready to use now (zero-config plus integrations you have configured). ' @@ -181,6 +250,20 @@ def register(registry: ToolRegistry) -> None: }, )(_describe_component) + registry.register( + 'resolve_config', + 'Show what a component config resolves to at load: the engine applies profile and default ' + 'merging, so the .pipe rarely says what the node receives. Reports discarded keys.', + { + 'type': 'object', + 'properties': { + 'provider': {'type': 'string', 'description': 'Component provider, e.g. llm_openai'}, + 'config': {'type': 'object', 'description': "The component's config block; omit for defaults"}, + }, + 'required': ['provider'], + }, + )(_resolve_config) + registry.register( 'validate_pipeline', "Validate a pipeline against the engine's own rules (zero client-side rules -- zero drift).", diff --git a/packages/ai/src/ai/modules/mcp/tools/scaffold.py b/packages/ai/src/ai/modules/mcp/tools/scaffold.py new file mode 100644 index 000000000..f19d71bba --- /dev/null +++ b/packages/ai/src/ai/modules/mcp/tools/scaffold.py @@ -0,0 +1,259 @@ +# Copyright 2026 Aparavi Software AG. MIT License. +"""Node scaffolding: emit a local node that loads on the first try. + +The literals here follow the tree rather than the docs, because the documented +contract is wrong in three places: + +- ``preconfig`` is marked optional in README-node-schema.md, but + ``Config.getNodeConfig`` raises without it and nearly every ``IGlobal`` calls + that at load, so a node without it hard-fails. +- ``depends()`` is documented as running in ``__init__.py``. The real nodes call + it from ``IGlobal.beginGlobal``, and third-party imports must follow it. +- The "Adding a New Node" example shows a plain class with a ``process()`` + method, which nothing in the engine calls. + +Two the docs omit: a ``services.json`` without ``protocol`` is not registered as +a service at all, and ``register`` must be ``filter`` or ``endpoint`` or the node +loads but can never be instantiated. +""" + +import keyword +import re +from typing import Any, Dict + +from ..errors import _bad +from ..tooling import ToolRegistry +from ._common import engine_call + +# Handler and argument per input lane, so the skeleton compiles for the lane it +# was asked for instead of always assuming text. +_LANE_HANDLERS = { + 'text': ('writeText', 'text: str'), + 'documents': ('writeDocuments', 'documents: list'), + 'questions': ('writeQuestions', 'question'), + 'answers': ('writeAnswers', 'answer'), + 'table': ('writeTable', 'table: str'), + 'json': ('writeJson', 'data'), + 'tags': ('writeTag', 'tag'), +} + +_NAME_RE = re.compile(r'^[a-z][a-z0-9_]*$') + +_SERVICES_JSON = """{{ +\t"title": "{title}", +\t"protocol": "{name}://", +\t"classType": ["{class_type}"], +\t"capabilities": [], +\t"register": "filter", +\t"node": "python", +\t"path": "local_nodes.{name}", +\t"prefix": "{name}", +\t"description": ["TODO: describe what this node does."], +\t"documentation": "https://docs.rocketride.org", +\t"lanes": {{ +\t\t"{lane_in}": ["{lane_out}"] +\t}}, +\t"preconfig": {{ +\t\t"default": "default", +\t\t"profiles": {{ +\t\t\t"default": {{}} +\t\t}} +\t}}, +\t"fields": {{}}, +\t"shape": [] +}} +""" + +_INIT_PY = """from .IGlobal import IGlobal +from .IInstance import IInstance + +__all__ = [ + 'IGlobal', + 'IInstance', +] +""" + +_PARENT_INIT_PY = '# Marks local_nodes as a package so the engine can import local_nodes..\n' + +_REQUIREMENTS = '# One pinned dependency per line, installed by depends() in IGlobal.beginGlobal.\n' + +_IGLOBAL_PY = '''import os + +from rocketlib import IGlobalBase, OPEN_MODE + +from ai.common.config import Config + + +class IGlobal(IGlobalBase): + """Per-pipeline state for the {name} node.""" + + def beginGlobal(self): + """Install dependencies and resolve config once per pipeline run.""" + if self.IEndpoint.endpoint.openMode == OPEN_MODE.CONFIG: + # Config mode only asks for the schema, so the driver is not needed. + return + + from depends import depends # type: ignore + + # depends() runs here rather than in __init__.py, and any third-party + # import has to come after it or the first load fails. + requirements = os.path.dirname(os.path.realpath(__file__)) + '/requirements.txt' + depends(requirements) + + self.config = Config.getNodeConfig(self.glb.logicalType, self.glb.connConfig) + + def endGlobal(self): + """Release whatever beginGlobal acquired.""" + self.config = None +''' + +_IINSTANCE_PY = '''from rocketlib import IInstanceBase + +from .IGlobal import IGlobal + + +class IInstance(IInstanceBase): + """Per-object handler for the {name} node.""" + + IGlobal: IGlobal + + def {handler}(self, {arg}): + """TODO: process the incoming {lane_in} and forward the result. + + The engine runs its own forward after this returns unless + preventDefault() raises, so forwarding explicitly and returning normally + delivers twice. Keep preventDefault() last if this forwards, and drop it + entirely if this only mutates in place. + """ + self.instance.{handler}({arg_name}) + return self.preventDefault() +''' + + +def _class_types(services: Dict[str, Any]) -> set: + """Every classType in use across the catalog, so the allowed set cannot drift.""" + found = set() + for service in (services or {}).values(): + types = service.get('classType') if isinstance(service, dict) else None + if isinstance(types, list): + found.update(v for v in types if isinstance(v, str)) + return found + + +def _lane_names(services: Dict[str, Any]) -> set: + """Every lane name in use, on either side of a catalog lane map.""" + found = set() + for service in (services or {}).values(): + lanes = service.get('lanes') if isinstance(service, dict) else None + if not isinstance(lanes, dict): + continue + found.update(k for k in lanes if isinstance(k, str)) + for outs in lanes.values(): + if isinstance(outs, list): + found.update(v for v in outs if isinstance(v, str)) + return found + + +def _defaulted(args: Dict[str, Any], key: str, fallback: Any) -> Any: + """Return the supplied argument, falling back only when it is absent or null. + + `or` would coerce a supplied falsy value such as 0 or '' into the fallback and + hide it from the type check, which is the mistake worth reporting. + """ + value = args.get(key) + return fallback if value is None else value + + +async def _scaffold_node(client, tasks, args: Dict[str, Any]) -> dict: + name = args.get('name') + if not name: + return _bad('name is required', 'pick a lowercase identifier, e.g. my_node') + # Dispatch does not apply the tool's inputSchema, so a non-string arrives intact + # and would reach the regex as a TypeError instead of an actionable answer. + if not isinstance(name, str) or not _NAME_RE.match(name) or keyword.iskeyword(name): + return _bad( + f'name must be a lowercase Python identifier, got {name!r}', + 'the engine imports local_nodes., so it has to be importable', + ) + + lane_in = _defaulted(args, 'lane_in', 'text') + lane_out = _defaulted(args, 'lane_out', lane_in) + class_type = _defaulted(args, 'class_type', lane_in) + for label, value in (('lane_in', lane_in), ('lane_out', lane_out), ('class_type', class_type)): + if not isinstance(value, str): + return _bad(f'{label} must be a string, got {value!r}', 'call list_components for the names in use') + + if lane_in not in _LANE_HANDLERS: + return _bad( + f'no handler template for lane {lane_in!r}', + f'supported lanes: {", ".join(sorted(_LANE_HANDLERS))}', + ) + + services, err = await engine_call(client.get_services(), 'scaffold_node') + if err: + return err + + # Validate against the live catalog so the allowed sets follow the engine + # rather than a list here that drifts as nodes are added. + catalog = (services or {}).get('services') or {} + known_types = _class_types(catalog) + if known_types and class_type not in known_types: + return _bad( + f'unknown class_type {class_type!r}', + f'call list_components, or use one in service today: {", ".join(sorted(known_types))}', + ) + + known_lanes = _lane_names(catalog) + for label, lane in (('lane_in', lane_in), ('lane_out', lane_out)): + if known_lanes and lane not in known_lanes: + return _bad( + f'unknown {label} {lane!r}', + f'lanes in service today: {", ".join(sorted(known_lanes))}', + ) + + handler, arg = _LANE_HANDLERS[lane_in] + title = name.replace('_', ' ').title() + + files = { + 'local_nodes/__init__.py': _PARENT_INIT_PY, + f'local_nodes/{name}/__init__.py': _INIT_PY, + f'local_nodes/{name}/services.json': _SERVICES_JSON.format( + title=title, name=name, class_type=class_type, lane_in=lane_in, lane_out=lane_out + ), + f'local_nodes/{name}/IGlobal.py': _IGLOBAL_PY.format(name=name), + f'local_nodes/{name}/IInstance.py': _IINSTANCE_PY.format( + name=name, handler=handler, arg=arg, arg_name=arg.split(':')[0], lane_in=lane_in + ), + f'local_nodes/{name}/requirements.txt': _REQUIREMENTS, + } + + return { + 'ok': True, + 'name': name, + 'provider': name, + 'files': files, + 'next_steps': [ + 'Write these files under the workspace passed as --node_path.', + f'Reference the node in a .pipe as "provider": "{name}", since the provider is the protocol.', + 'Restart the engine: node manifests are read once at startup.', + ], + } + + +def register(registry: ToolRegistry) -> None: + """Register the scaffolding tool against ``registry``.""" + registry.register( + 'scaffold_node', + 'Emit a local node skeleton that loads on the first try, with the manifest keys and file ' + 'layout the engine actually requires. Returns files to write; it writes nothing itself.', + { + 'type': 'object', + 'properties': { + 'name': {'type': 'string', 'description': 'Lowercase identifier, e.g. my_node'}, + 'lane_in': {'type': 'string', 'description': 'Input lane the node handles; defaults to text'}, + 'lane_out': {'type': 'string', 'description': 'Output lane it emits; defaults to lane_in'}, + 'class_type': {'type': 'string', 'description': 'Component class; defaults to lane_in'}, + }, + 'required': ['name'], + }, + )(_scaffold_node) diff --git a/packages/ai/src/ai/modules/task/commands/cmd_misc.py b/packages/ai/src/ai/modules/task/commands/cmd_misc.py index aa3a1f071..630febfac 100644 --- a/packages/ai/src/ai/modules/task/commands/cmd_misc.py +++ b/packages/ai/src/ai/modules/task/commands/cmd_misc.py @@ -49,6 +49,7 @@ from typing import TYPE_CHECKING, Dict, Any, List, Tuple from rocketride import EVENT_TYPE from rocketlib import validatePipeline +from ai.common.config import Config from ai.common.dap import DAPConn, TransportBase from ai.common.list_rows import paginate_rows from ai.account.models import resolve_task_permissions @@ -237,6 +238,73 @@ async def on_rrext_validate(self, request: Dict[str, Any]) -> Dict[str, Any]: self.debug_message(f'Pipeline validation failed: {str(e)}') raise + async def on_rrext_resolve_config(self, request: Dict[str, Any]) -> Dict[str, Any]: + """ + Handle DAP 'rrext_resolve_config' to resolve a component config as a node sees it. + + Runs the config through the same ``Config.getNodeConfig`` a node calls at + load, so an author can see what the node actually receives rather than + what the .pipe appears to say. This has to happen engine-side: the + service catalog does not carry ``preconfig``, so profile resolution + cannot be reproduced from ``rrext_services``. + + Args: + request (Dict[str, Any]): DAP request containing: + - arguments (Dict[str, Any]): + - provider (str): Component provider, e.g. 'llm_openai'. + - config (Dict[str, Any], optional): The component's config block. + + Returns: + Dict[str, Any]: DAP response whose body carries: + - provider (str): The provider that was resolved. + - profile (str): The profile that applied, named or default. + - resolved (Dict[str, Any]): What the node receives. + - dropped (List[str]): Top-level config keys the resolver discarded. + + Raises: + ValueError: If provider is missing or config is not an object. + Exception: If the service is unknown or has no preconfig section. + """ + try: + args = request.get('arguments', {}) + provider = args.get('provider') + if not provider: + raise ValueError('provider is required') + + # Default only a genuinely absent config: `or {}` would coerce a + # falsy non-object such as [] and skip the type check below. + config = args.get('config') + if config is None: + config = {} + if not isinstance(config, dict): + raise ValueError('config must be an object') + + resolved = Config.getNodeConfig(provider, config) + profile = config.get('profile') + + # Report the keys the resolver discarded rather than leaving the author + # to infer it from an absence. With a profile set, getNodeConfig reads + # the user layer only from the sub-object named after that profile, so + # sibling top-level keys never reach the node (#1839). + dropped = [] + if profile: + # Every sibling is discarded, so the value is not worth comparing: one + # that happens to match the profile's own is still a line the resolver + # never read, and staying quiet about it is what hides the bug. + dropped = [k for k in config if k not in ('profile', profile)] + + body = { + 'provider': provider, + 'profile': profile or 'default', + 'resolved': resolved, + 'dropped': dropped, + } + return self.build_response(request, body=body) + + except Exception as e: + self.debug_message(f'Config resolution failed for {request.get("arguments", {}).get("provider")}: {str(e)}') + raise + async def on_rrext_dashboard(self, request: Dict[str, Any]) -> Dict[str, Any]: """ Handle DAP 'rrext_dashboard' command to retrieve server dashboard data. diff --git a/packages/ai/tests/ai/modules/mcp/conftest.py b/packages/ai/tests/ai/modules/mcp/conftest.py index 0e4c26ccc..65680e444 100644 --- a/packages/ai/tests/ai/modules/mcp/conftest.py +++ b/packages/ai/tests/ai/modules/mcp/conftest.py @@ -17,6 +17,7 @@ # introspection 'list_components', 'describe_component', + 'resolve_config', 'validate_pipeline', 'describe_pipeline', # execution @@ -45,6 +46,8 @@ 'log_read', 'log_traces', 'log_trace', + # scaffold + 'scaffold_node', # integrations 'list_integrations', ) @@ -60,6 +63,7 @@ def __init__( services=None, service_defs=None, validate_result=None, + resolve_config_result=None, task_statuses=None, public_token='pub-1', base_url='http://localhost:5565', @@ -135,6 +139,8 @@ def __init__( ) self._service_defs = service_defs if service_defs is not None else dict(self._services.get('services', {})) self._validate_result = validate_result if validate_result is not None else {'errors': [], 'warnings': []} + self._resolve_config_result = resolve_config_result or {} + self.resolve_config_calls = [] self.get_services_calls = 0 self.get_service_calls = [] self.validate_calls = [] @@ -182,6 +188,10 @@ async def validate(self, pipeline, source=None): self.validate_calls.append({'pipeline': pipeline, 'source': source}) return dict(self._validate_result) + async def resolve_config(self, provider, config=None): + self.resolve_config_calls.append({'provider': provider, 'config': config}) + return dict(self._resolve_config_result) + async def use(self, **kwargs): self.used.append(kwargs) return { diff --git a/packages/ai/tests/ai/modules/mcp/test_handlers.py b/packages/ai/tests/ai/modules/mcp/test_handlers.py index 22804fbe4..822dac5c3 100644 --- a/packages/ai/tests/ai/modules/mcp/test_handlers.py +++ b/packages/ai/tests/ai/modules/mcp/test_handlers.py @@ -3,7 +3,7 @@ the resource wiring it keeps (status / pipelines, no nodes). Dispatch tests inject a dummy tool by monkeypatching `tools_pkg.register_all`, -isolating the dispatch machinery from the real 27-tool surface (which +isolating the dispatch machinery from the real 29-tool surface (which `test_list_tools_reflects_real_register_all` covers against `conftest.EXPECTED_TOOL_NAMES`). diff --git a/packages/ai/tests/ai/modules/mcp/test_introspection.py b/packages/ai/tests/ai/modules/mcp/test_introspection.py index c76ac82f9..aa6ef941a 100644 --- a/packages/ai/tests/ai/modules/mcp/test_introspection.py +++ b/packages/ai/tests/ai/modules/mcp/test_introspection.py @@ -530,3 +530,147 @@ async def _hang(*args, **kwargs): assert result['ok'] is False assert result['error_type'] == 'Timeout' + + +# --------------------------------------------------------------------------- +# resolve_config +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_resolve_config_passes_provider_and_config_through(fake_engine): + """The tool is a thin pass-through: resolution has to happen engine-side.""" + fake_engine._resolve_config_result = { + 'provider': 'llm_openai', + 'profile': 'default', + 'resolved': {'model': 'gpt-4o'}, + 'dropped': [], + } + registry = ToolRegistry() + introspection.register(registry) + + result = await registry.handler('resolve_config')( + fake_engine, None, {'provider': 'llm_openai', 'config': {'model': 'gpt-4o'}} + ) + + assert result['ok'] is True + assert result['resolved'] == {'model': 'gpt-4o'} + assert fake_engine.resolve_config_calls == [{'provider': 'llm_openai', 'config': {'model': 'gpt-4o'}}] + + +@pytest.mark.asyncio +async def test_resolve_config_explains_keys_dropped_by_a_profile(fake_engine): + """ + The day-losing case from #1989: a key beside 'profile' never reaches the node. + + An absence is not self-explanatory, so the tool names the discarded keys and + says where to put them instead. + """ + fake_engine._resolve_config_result = { + 'provider': 'store_pinecone', + 'profile': 'serverless-dense', + 'resolved': {'collection': 'ROCKETRIDE'}, + 'dropped': ['apikey', 'pipeline_path'], + } + registry = ToolRegistry() + introspection.register(registry) + + result = await registry.handler('resolve_config')( + fake_engine, + None, + {'provider': 'store_pinecone', 'config': {'profile': 'serverless-dense', 'apikey': 'x'}}, + ) + + assert result['dropped'] == ['apikey', 'pipeline_path'] + assert 'apikey' in result['hint'] and 'pipeline_path' in result['hint'] + assert 'serverless-dense' in result['hint'], 'the hint must say which object to move them into' + + +@pytest.mark.asyncio +async def test_resolve_config_stays_quiet_when_nothing_was_dropped(fake_engine): + """No hint when there is nothing to correct, so the hint stays meaningful.""" + fake_engine._resolve_config_result = {'provider': 'ocr', 'profile': 'default', 'resolved': {}, 'dropped': []} + registry = ToolRegistry() + introspection.register(registry) + + result = await registry.handler('resolve_config')(fake_engine, None, {'provider': 'ocr'}) + + assert 'hint' not in result + + +@pytest.mark.asyncio +async def test_resolve_config_rejects_a_missing_provider(fake_engine): + registry = ToolRegistry() + introspection.register(registry) + + result = await registry.handler('resolve_config')(fake_engine, None, {}) + + assert result['ok'] is False + assert result['error_type'] == 'BadRequest' + assert fake_engine.resolve_config_calls == [], 'a bad request must not reach the engine' + + +@pytest.mark.asyncio +async def test_resolve_config_rejects_a_non_object_config(fake_engine): + registry = ToolRegistry() + introspection.register(registry) + + result = await registry.handler('resolve_config')(fake_engine, None, {'provider': 'ocr', 'config': 'nope'}) + + assert result['ok'] is False + assert fake_engine.resolve_config_calls == [] + + +@pytest.mark.asyncio +async def test_validate_pipeline_reports_a_provider_the_engine_does_not_have(fake_engine): + """The engine validates a pipeline structurally and never checks its providers. + + A typed provider name passes validation and fails later at run time, which is + the gap this tool exists to close. + """ + registry = ToolRegistry() + introspection.register(registry) + fake_engine._services = {'services': {'parse': {}, 'llm_openai': {}}} + pipeline = { + 'components': [ + {'id': 'a', 'provider': 'parse', 'config': {}}, + {'id': 'b', 'provider': 'no_such_provider', 'config': {}}, + ] + } + + result = await registry.handler('validate_pipeline')(fake_engine, None, {'pipeline': pipeline}) + + assert result['ok'] is False + assert [e['component'] for e in result['errors']] == ['b'] + assert 'no_such_provider' in result['errors'][0]['message'] + + +@pytest.mark.asyncio +async def test_validate_pipeline_accepts_providers_in_the_catalog(fake_engine): + """Every provider present means the added check contributes no errors.""" + registry = ToolRegistry() + introspection.register(registry) + fake_engine._services = {'services': {'parse': {}, 'llm_openai': {}}} + pipeline = {'components': [{'id': 'a', 'provider': 'parse', 'config': {}}]} + + result = await registry.handler('validate_pipeline')(fake_engine, None, {'pipeline': pipeline}) + + assert result['ok'] is True + assert result['errors'] == [] + + +@pytest.mark.asyncio +async def test_validate_pipeline_keeps_engine_errors_alongside_provider_errors(fake_engine): + """The engine's own findings must survive, not be replaced by the added check.""" + registry = ToolRegistry() + introspection.register(registry) + fake_engine._services = {'services': {'parse': {}}} + fake_engine._validate_result = {'errors': [{'ccode': 40, 'message': 'structural'}], 'warnings': ['w']} + pipeline = {'components': [{'id': 'b', 'provider': 'no_such_provider', 'config': {}}]} + + result = await registry.handler('validate_pipeline')(fake_engine, None, {'pipeline': pipeline}) + + messages = [e.get('message') for e in result['errors']] + assert 'structural' in messages + assert any('no_such_provider' in m for m in messages) + assert result['warnings'] == ['w'] diff --git a/packages/ai/tests/ai/modules/mcp/test_scaffold.py b/packages/ai/tests/ai/modules/mcp/test_scaffold.py new file mode 100644 index 000000000..35d44c8e7 --- /dev/null +++ b/packages/ai/tests/ai/modules/mcp/test_scaffold.py @@ -0,0 +1,188 @@ +# Copyright 2026 Aparavi Software AG. MIT License. +"""Tests for the node scaffolding tool (`tools/scaffold.py`). + +The tool exists because the documented node contract is wrong in several +places, so these assert the emitted skeleton against the engine's real +requirements rather than against the docs. +""" + +import ast +import json + +import pytest + +from ai.modules.mcp.tooling import ToolRegistry +from ai.modules.mcp.tools import scaffold + + +def _registry(): + registry = ToolRegistry() + scaffold.register(registry) + return registry + + +async def _scaffold(fake_engine, **args): + args.setdefault('name', 'my_node') + return await _registry().handler('scaffold_node')(fake_engine, None, args) + + +@pytest.fixture +def catalog_engine(fake_engine): + """A fake engine whose catalog covers the lanes and class types these tests use.""" + fake_engine._services = { + 'services': { + 'question': {'classType': ['text'], 'lanes': {'text': ['questions']}}, + 'ocr': {'classType': ['image'], 'lanes': {'image': ['text', 'documents']}}, + 'ner': {'classType': ['documents'], 'lanes': {'documents': ['documents']}}, + } + } + return fake_engine + + +@pytest.mark.asyncio +async def test_every_generated_python_file_compiles(catalog_engine): + """A skeleton that does not import is worse than no skeleton.""" + result = await _scaffold(catalog_engine) + + py = {p: c for p, c in result['files'].items() if p.endswith('.py')} + assert len(py) == 4, f'expected the parent init, the node init, IGlobal and IInstance, got {sorted(py)}' + for path, content in py.items(): + ast.parse(content) # raises SyntaxError if the template is malformed + + +@pytest.mark.asyncio +async def test_manifest_carries_the_keys_the_engine_actually_requires(catalog_engine): + """ + The four traps that stop a node loading, none of them stated correctly in the docs. + + preconfig is documented optional but Config.getNodeConfig raises without it; + a manifest without protocol is never registered as a service; register must + name a factory type; and the import path is local_nodes.. + """ + result = await _scaffold(catalog_engine, name='my_node') + + manifest = json.loads(result['files']['local_nodes/my_node/services.json']) + + assert manifest['protocol'] == 'my_node://' + assert manifest['register'] == 'filter' + assert manifest['path'] == 'local_nodes.my_node' + assert 'preconfig' in manifest, 'documented optional, but getNodeConfig raises without it' + assert manifest['preconfig']['default'] in manifest['preconfig']['profiles'], ( + 'the default profile must name a profile that exists, or resolution raises at load' + ) + + +@pytest.mark.asyncio +async def test_the_provider_is_the_protocol(catalog_engine): + """Naming a component by its title instead of its protocol is the issue's own example.""" + result = await _scaffold(catalog_engine, name='my_node') + + assert result['provider'] == 'my_node' + assert any('provider is the protocol' in step for step in result['next_steps']) + + +@pytest.mark.asyncio +async def test_the_parent_package_marker_is_included(catalog_engine): + """The engine imports local_nodes., so the parent needs to be a package.""" + result = await _scaffold(catalog_engine) + + assert 'local_nodes/__init__.py' in result['files'] + + +@pytest.mark.asyncio +async def test_depends_is_called_from_iglobal_not_init(catalog_engine): + """README-nodes.md puts depends() in __init__.py and contradicts itself later.""" + result = await _scaffold(catalog_engine, name='my_node') + + assert 'depends(' in result['files']['local_nodes/my_node/IGlobal.py'] + assert 'depends(' not in result['files']['local_nodes/my_node/__init__.py'] + + +@pytest.mark.asyncio +async def test_the_handler_matches_the_requested_lane(catalog_engine): + """A text skeleton for a documents node would not run.""" + result = await _scaffold(catalog_engine, name='doc_node', lane_in='documents', lane_out='documents') + + instance = result['files']['local_nodes/doc_node/IInstance.py'] + assert 'def writeDocuments(self, documents: list):' in instance + assert 'self.instance.writeDocuments(documents)' in instance + + +@pytest.mark.asyncio +async def test_class_type_is_checked_against_the_live_catalog(catalog_engine): + """The allowed set follows the engine, so it cannot drift as nodes are added.""" + result = await _scaffold(catalog_engine, class_type='not_a_class') + + assert result['ok'] is False + assert 'not_a_class' in result['message'] + assert 'text' in result['hint'], 'the hint should name what is actually available' + + +@pytest.mark.asyncio +async def test_a_name_that_is_not_importable_is_rejected(catalog_engine): + """The engine imports local_nodes., so a bad name fails at load, not here.""" + for bad in ('My-Node', '9lives', 'class', 'my node'): + result = await _scaffold(catalog_engine, name=bad) + assert result['ok'] is False, f'{bad!r} should be rejected' + + +@pytest.mark.asyncio +async def test_an_unsupported_lane_is_refused_rather_than_guessed(catalog_engine): + """Emitting a handler for a lane with no template would produce a node that cannot run.""" + result = await _scaffold(catalog_engine, lane_in='audio') + + assert result['ok'] is False + assert 'audio' in result['message'] + + +@pytest.mark.asyncio +async def test_the_tool_writes_nothing_itself(catalog_engine): + """Files come back for the caller to write, matching load_pipeline's stance on reads.""" + result = await _scaffold(catalog_engine) + + assert isinstance(result['files'], dict) + assert all(isinstance(c, str) for c in result['files'].values()) + + +@pytest.mark.asyncio +async def test_a_malformed_class_type_entry_is_ignored(fake_engine): + """A catalog entry holding a bare string must not widen the allowed set. + + Iterating it yields characters, so 'text' would admit 't', 'e' and 'x' as + class types and reject the real one. + """ + fake_engine._services = { + 'services': { + 'broken': {'classType': 'text', 'lanes': {'text': ['text']}}, + 'good': {'classType': ['documents'], 'lanes': {'text': ['text']}}, + } + } + + assert scaffold._class_types(fake_engine._services['services']) == {'documents'} + + result = await _scaffold(fake_engine, lane_in='text', class_type='x') + assert result['ok'] is False, 'a character from a malformed entry is not a class type' + + +@pytest.mark.asyncio +async def test_a_non_string_argument_is_refused_rather_than_raised(catalog_engine): + """Dispatch does not apply inputSchema, so arguments arrive with any type.""" + result = await _scaffold(catalog_engine, name=123) + assert result['ok'] is False + assert 'identifier' in result['message'] + + # Falsy values included: `or` defaulting would swallow these before the check. + for field in ('lane_in', 'lane_out', 'class_type'): + for value in (42, 0, [], False): + result = await _scaffold(catalog_engine, **{field: value}) + assert result['ok'] is False, f'{field} accepted {value!r}' + assert field in result['message'] + + +@pytest.mark.asyncio +async def test_lane_in_is_checked_against_the_live_catalog(catalog_engine): + """A lane with a local template still has to be one the engine serves.""" + result = await _scaffold(catalog_engine, lane_in='table', lane_out='text', class_type='text') + + assert result['ok'] is False + assert 'lane_in' in result['message'], result diff --git a/packages/ai/tests/ai/modules/task/commands/test_cmd_misc.py b/packages/ai/tests/ai/modules/task/commands/test_cmd_misc.py index 316cbe2ca..ad893aa74 100644 --- a/packages/ai/tests/ai/modules/task/commands/test_cmd_misc.py +++ b/packages/ai/tests/ai/modules/task/commands/test_cmd_misc.py @@ -838,3 +838,135 @@ def test_misc_commands_init_is_noop(): """The mixin's __init__ accepts the standard arguments without setting state.""" instance = MiscCommands.__new__(MiscCommands) MiscCommands.__init__(instance, connection_id=1, server=None, transport=None) + + +# --------------------------------------------------------------------------- +# on_rrext_resolve_config +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_returns_what_the_node_receives(monkeypatch): + """Resolution runs through the engine's own getNodeConfig, not a reimplementation.""" + monkeypatch.setattr(cmd_misc.Config, 'getNodeConfig', staticmethod(lambda p, c: {'model': 'gpt-4o', 'temp': 0})) + + conn = _make_conn() + request = {'arguments': {'provider': 'llm_openai', 'config': {'model': 'gpt-4o'}}} + result = await MiscCommands.on_rrext_resolve_config(conn, request) + + body = result['body'] + assert body['provider'] == 'llm_openai' + assert body['profile'] == 'default' + assert body['resolved'] == {'model': 'gpt-4o', 'temp': 0} + assert body['dropped'] == [] + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_reports_keys_a_profile_discards(monkeypatch): + """ + The #1839 shape: with a profile set, sibling top-level keys never reach the node. + + Reporting them is the point of the tool. Inferring it from an absence is what + cost the issue author a day. + """ + monkeypatch.setattr(cmd_misc.Config, 'getNodeConfig', staticmethod(lambda p, c: {'host': 'localhost'})) + + conn = _make_conn() + request = { + 'arguments': { + 'provider': 'store_chroma', + 'config': {'profile': 'local', 'apikey': 'sk-x', 'local': {'host': 'localhost'}}, + }, + } + result = await MiscCommands.on_rrext_resolve_config(conn, request) + + body = result['body'] + assert body['profile'] == 'local' + assert body['dropped'] == ['apikey'], 'the profile sub-object itself is not a dropped key' + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_reports_a_key_the_profile_overwrote(monkeypatch): + """The discard that hides itself: the profile defines the key, so it stays present. + + Reporting only absent keys misses this, which is the common shape in the + catalog: nearly every profile declares apikey, so an apikey written beside + 'profile' is silently replaced rather than dropped from the result. + """ + monkeypatch.setattr(cmd_misc.Config, 'getNodeConfig', staticmethod(lambda p, c: {'apikey': '', 'model': 'gpt-5'})) + + conn = _make_conn() + request = { + 'arguments': { + 'provider': 'llm_openai', + 'config': {'profile': 'openai-5-4', 'apikey': 'sk-authors-key'}, + }, + } + result = await MiscCommands.on_rrext_resolve_config(conn, request) + + body = result['body'] + assert body['resolved']['apikey'] == '', 'the profile value wins, which is the bug being surfaced' + assert body['dropped'] == ['apikey'], 'present-but-overwritten still means the author key never lands' + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_reports_a_sibling_that_matches_the_profile(monkeypatch): + """A sibling whose value coincides with the profile's is still never read. + + Comparing values would stay quiet here and leave the author believing the + line is in effect, when the same key inside the profile is what applied. + """ + monkeypatch.setattr(cmd_misc.Config, 'getNodeConfig', staticmethod(lambda p, c: {'model': 'gpt-5.4'})) + + conn = _make_conn() + request = { + 'arguments': { + 'provider': 'llm_openai', + 'config': {'profile': 'openai-5-4', 'model': 'gpt-5.4'}, + }, + } + result = await MiscCommands.on_rrext_resolve_config(conn, request) + + assert result['body']['dropped'] == ['model'] + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_reports_nothing_without_a_profile(monkeypatch): + """Without a profile the user layer is read from the top level, so nothing is lost.""" + monkeypatch.setattr(cmd_misc.Config, 'getNodeConfig', staticmethod(lambda p, c: {'model': 'gpt-5.4'})) + + conn = _make_conn() + request = {'arguments': {'provider': 'llm_openai', 'config': {'model': 'gpt-5.4'}}} + result = await MiscCommands.on_rrext_resolve_config(conn, request) + + assert result['body']['dropped'] == [] + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_requires_a_provider(): + conn = _make_conn() + + with pytest.raises(ValueError): + await MiscCommands.on_rrext_resolve_config(conn, {'arguments': {}}) + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_rejects_a_non_object_config(): + conn = _make_conn() + + with pytest.raises(ValueError): + await MiscCommands.on_rrext_resolve_config(conn, {'arguments': {'provider': 'ocr', 'config': []}}) + + +@pytest.mark.asyncio +async def test_on_rrext_resolve_config_propagates_an_unknown_service(monkeypatch): + """An unknown service raises out of getNodeConfig; the caller should see that, not a blank.""" + + def _raise(provider, config): + raise Exception(f'The service {provider} was not found') + + monkeypatch.setattr(cmd_misc.Config, 'getNodeConfig', staticmethod(_raise)) + + conn = _make_conn() + with pytest.raises(Exception, match='was not found'): + await MiscCommands.on_rrext_resolve_config(conn, {'arguments': {'provider': 'nope'}}) diff --git a/packages/client-python/src/rocketride/mixins/services.py b/packages/client-python/src/rocketride/mixins/services.py index ac35e9e9d..a996a411a 100644 --- a/packages/client-python/src/rocketride/mixins/services.py +++ b/packages/client-python/src/rocketride/mixins/services.py @@ -105,6 +105,33 @@ async def get_service(self, service: str) -> SERVICE_DEFINITION: return await self.call('rrext_services', service=service) + async def resolve_config(self, provider: str, config: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + """ + Resolve a component config the way the node will receive it. + + Applies the engine's own profile and default merging, so an author can + see the effective config instead of inferring it. Resolution is + engine-side because the service catalog does not carry ``preconfig``. + + Args: + provider: Component provider, e.g. 'llm_openai'. + config: The component's config block. Defaults to empty. + + Returns: + ``{'provider', 'profile', 'resolved', 'dropped'}``, where ``dropped`` + lists top-level keys the resolver discarded. + + Raises: + ValueError: If provider is empty. + RuntimeError: If the service is unknown or has no preconfig section. + """ + if not provider: + raise ValueError('Provider name is required') + + # Default only an absent config: `or {}` would coerce a falsy non-object + # such as [] and hide it from the engine's type check. + return await self.call('rrext_resolve_config', provider=provider, config={} if config is None else config) + async def validate( self, pipeline: PipelineConfig,