From 506bf15b6b5aa5ee34c8cae99d1524f1aebc3d58 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:05:36 +0200 Subject: [PATCH 1/3] feat(publish): NL-description -> IntegrationSpec ingress (opt-in LLM, aix-backed) The second PUBLISH ingress arm (landscape doc 9.2): refine a natural-language description into a *draft* IntegrationSpec via oa's prompt-as-function machinery, routed through aix (provider-agnostic) -- NOT oa's OpenAI default -- to keep the multi-target promise honest. Kept strictly opt-in (DECISIONS D18): - coact/nl_ingress.py: integration_spec_from_description(description, *, llm=None, model=None, name=None, infer_tool_schemas=True). Backend injectable (callable / model-name / None -> aix.chat); oa/aix imported LAZILY so importing coact stays provider-free (D10, regression-tested in a subprocess). - IntegrationSpec grows tool_specs: list[ToolSpec] (name/description/input_schema/ handler) alongside the unchanged tools refs (non-breaking; landscape 9.1). A ToolSpec with a handler is bound (runnable); without, it is a proposed draft. Added runnable_refs() + render(); is_empty() now counts tool_specs. - publish_mcpb builds the server config from runnable_refs(); a pure draft (no runnable ref) RAISES with guidance instead of writing a dead bundle; proposed tools are listed in the manifest with a warning. - per-tool input-schema inference via oa.infer_schema_from_verbal_description, made backend-injectable upstream (oa PR; first customer = coact). - authoring prompt SSOT in coact (DFLT_AUTHORING_PROMPTS), injectable via prompt_template= (pyrompt = the iteration home, not a hard dep). - CLI verb 'coact describe'; exports ToolSpec + integration_spec_from_description; extra coact[nl] (oa, aix); skill + README updated. - 17 offline tests (fake backend, no provider call). Full suite: 380 passed. --- .claude/skills/coact-publish/SKILL.md | 38 +++- README.md | 19 ++ coact/__init__.py | 5 +- coact/__main__.py | 40 +++- coact/integration.py | 87 +++++++- coact/nl_ingress.py | 282 ++++++++++++++++++++++++++ coact/publish_mcpb.py | 35 +++- misc/docs/DECISIONS.md | 60 ++++++ pyproject.toml | 8 + tests/test_cli.py | 30 ++- tests/test_nl_ingress.py | 216 ++++++++++++++++++++ 11 files changed, 805 insertions(+), 15 deletions(-) create mode 100644 coact/nl_ingress.py create mode 100644 tests/test_nl_ingress.py diff --git a/.claude/skills/coact-publish/SKILL.md b/.claude/skills/coact-publish/SKILL.md index 337813d..98b6083 100644 --- a/.claude/skills/coact-publish/SKILL.md +++ b/.claude/skills/coact-publish/SKILL.md @@ -8,11 +8,13 @@ description: >- connector / plugin / MCP server / .mcpb / Desktop Extension / "integration" from existing Python code or a skill — e.g. "make an mcpb", "package these functions for Claude", "turn this into a Claude extension/connector", "publish - a local MCP server", "wrap my tools as a Claude Desktop extension". For REMOTE - claude.ai connectors (HTTPS + OAuth) this is the wrong target — that surface is - not built yet (see Limitations). + a local MCP server", "wrap my tools as a Claude Desktop extension". Also use to + draft an integration from a natural-language description ("describe an + integration", "I want a Claude connector that can…") via `coact describe`. For + REMOTE claude.ai connectors (HTTPS + OAuth) this is the wrong target — that + surface is not built yet (see Limitations). metadata: - version: 0.1.0 + version: 0.2.0 --- # coact publish — Python capability → Claude integration @@ -48,6 +50,34 @@ Sources accepted (mix freely): `module:function` refs, a skill directory / `SKILL.md` (its `coact: mcp:` block supplies the refs), or — from Python — live callables and a prebuilt `IntegrationSpec`. +## From a natural-language description (opt-in LLM) + +To go from an English description to a *draft* integration (proposed tools with +inferred input schemas), use `coact describe` — the **only** LLM-using path here +(the `module:function` → `.mcpb` path stays LLM-free). Generation routes through +`aix` (multi-provider) by default; `--llm` picks a model. + +```bash +coact describe "a connector that looks up the weather for a city and converts currencies" +# → renders a draft IntegrationSpec: proposed tools, each marked "proposed (no handler)" +``` + +```python +from coact import integration_spec_from_description, publish + +spec = integration_spec_from_description( + "expose os.path.basename and os.path.dirname as tools", name="paths" +) +# tools the description bound to existing code become runnable refs: +publish(spec, name="paths", dest="dist") # works iff spec.runnable_refs() +``` + +The draft is a **design artifact**: tools are *proposed* (no importable handler) +unless the description named existing code. **Bind** each proposed tool to a real +`module:function` handler (write the code, or point at existing functions) before +`publish` can build a runnable `.mcpb` — coact writes the design, you own the +code. Needs `coact[nl]` (`oa`, `aix`), imported lazily only on this path. + Python API: ```python diff --git a/README.md b/README.md index 1e158df..bb89949 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ coact inventory . # skills + agents + MC coact back .claude/agents/ux-analyst.md # lossy agent → skill stub coact scaffold .claude/agents/a.md .claude/agents/b.md # a starter fleet shim (you own it) coact publish mypkg.tools:summarize --name my-tools --dry-run # → a Claude .mcpb (preview) +coact describe "a tool that looks up the weather for a city" # NL → a draft IntegrationSpec ``` ## Publish — ship a capability to a chatbot host @@ -109,6 +110,24 @@ planned targets on the same open-closed registry. Background: [`misc/docs/CHATBOT_INTEGRATION_LANDSCAPE.md`](misc/docs/CHATBOT_INTEGRATION_LANDSCAPE.md). Install: `pip install coact[mcpb]`. +There are two ways to get an `IntegrationSpec`. The **mechanical** ingress above +(refs / callables / skills) uses **no LLM**. The **opt-in** ingress refines a +natural-language description into a *draft* spec — proposed tools with inferred +input schemas — routing generation through [`aix`](https://github.com/thorwhalen/aix) +(multi-provider) via [`oa`](https://github.com/thorwhalen/oa): + +```python +from coact import integration_spec_from_description + +spec = integration_spec_from_description("expose os.path.basename as a tool") +print(spec.render()) # tools the description bound to code become runnable refs +``` + +The draft is a **design artifact**: tools without a `module:function` handler are +*proposed* (won't run until you bind them to real code). The LLM touches **only** +this path — the code → `.mcpb` path stays LLM-free (`DECISIONS.md` D10/D18). +Install: `pip install coact[nl]`. + ## The model in one minute A `SKILL.md` is *procedural knowledge injected into the caller's turn*; a subagent diff --git a/coact/__init__.py b/coact/__init__.py index 89058fd..133ab8f 100644 --- a/coact/__init__.py +++ b/coact/__init__.py @@ -68,7 +68,8 @@ RunnableCrewAIAgent, realize_crewai, ) # registers 'crewai' -from coact.integration import IntegrationSpec, integration_spec_from +from coact.integration import IntegrationSpec, ToolSpec, integration_spec_from +from coact.nl_ingress import integration_spec_from_description from coact.publish import PublishResult, publish, publish_targets from coact.publish_mcpb import publish_mcpb # registers 'claude-local-mcpb' from coact.scaffold import scaffold_fleet @@ -125,7 +126,9 @@ def _resolve_version() -> str: "realization_backends", # PUBLISH (ship a capability to a chatbot host; Claude-local .mcpb first; D17) "IntegrationSpec", + "ToolSpec", "integration_spec_from", + "integration_spec_from_description", # opt-in LLM NL ingress (aix-backed; D18) "publish", "publish_targets", "PublishResult", diff --git a/coact/__main__.py b/coact/__main__.py index c082574..4029b5e 100644 --- a/coact/__main__.py +++ b/coact/__main__.py @@ -4,7 +4,8 @@ Mirrors ``skill``'s dispatch-to-interface pattern (CLI wrappers call the same core functions and format for the terminal), so the two packages feel like one toolkit. The verbs are ``plan``, ``complete``, ``emit``, ``realize``, ``diff``, -``estimate``, ``inventory``, ``back``, ``scaffold``, and ``publish``. Usage:: +``estimate``, ``inventory``, ``back``, ``scaffold``, ``publish``, and +``describe``. Usage:: python -m coact plan .claude/skills/ux-analyst python -m coact complete .claude/skills/ux-analyst --dest .claude/agents @@ -17,6 +18,7 @@ python -m coact back .claude/agents/ux-analyst.md python -m coact scaffold .claude/agents/a.md .claude/agents/b.md python -m coact publish my.module:my_func --dry-run + python -m coact describe "a tool that looks up the weather for a city" """ from __future__ import annotations @@ -158,10 +160,44 @@ def publish( return res.render() +def describe( + description: str, + *, + name: str | None = None, + llm: str | None = None, + author: str | None = None, +) -> str: + """Refine an NL description into a draft IntegrationSpec (opt-in LLM, aix-backed). + + The result is a *design draft*: proposed tools (with inferred input schemas). + Bind each tool to a ``module:function`` handler — or include such refs in the + description — then ``coact publish`` it. ``--llm`` is a model name (e.g. + ``gpt-4o``); the backend defaults to ``aix`` (multi-provider). + """ + from coact.nl_ingress import integration_spec_from_description + + spec = integration_spec_from_description( + description, llm=llm, name=name, author=author + ) + return spec.render() + + def main() -> None: """Dispatch the coact CLI.""" argh.dispatch_commands( - [plan, complete, emit, realize, diff, estimate, inventory, back, scaffold, publish] + [ + plan, + complete, + emit, + realize, + diff, + estimate, + inventory, + back, + scaffold, + publish, + describe, + ] ) diff --git a/coact/integration.py b/coact/integration.py index 88f1ff3..d99c606 100644 --- a/coact/integration.py +++ b/coact/integration.py @@ -32,13 +32,45 @@ ] +@dataclass +class ToolSpec: + """A richer, target-neutral description of one tool in an :class:`IntegrationSpec`. + + The *mechanical* ingress represents tools as bare ``'module:function'`` ref + strings (``IntegrationSpec.tools``). The *NL* ingress (:mod:`coact.nl_ingress`) + and the landscape-doc §9.1 model need more — a name, a description, an input + JSON Schema, and an **optional** handler ref: + + - A ToolSpec **with** a ``handler`` is *bound* (runnable): its ref joins the + spec's runnable set and the published server can import and call it. + - A ToolSpec **without** a handler is a *proposed* tool — a design draft to + bind to real code (or supply a ref for) before it can run. + + >>> ToolSpec(name='get_weather', handler='wx.api:current').is_bound() + True + >>> ToolSpec(name='get_weather').is_bound() + False + """ + + name: str + description: str = "" + input_schema: Optional[dict] = None + handler: Optional[str] = None # 'module:function' ref, or None if unbound + + def is_bound(self) -> bool: + """True when a ``module:function`` handler backs this tool (so it can run).""" + return bool(self.handler) + + @dataclass class IntegrationSpec: """Target-neutral description of an integration to publish. - The connectivity core maps onto MCP's three primitives. Only ``tools`` is - consumed by the local-``.mcpb`` target today; ``resources``/``prompts`` and - the ``auth``/``deployment`` hints are declared now (open-closed) for the + The connectivity core maps onto MCP's three primitives. Tools come in two + shapes that coexist: bare ``'module:function'`` refs in ``tools`` (the + mechanical/code ingress) and richer :class:`ToolSpec` descriptors in + ``tool_specs`` (the NL ingress, landscape-doc §9.1). ``resources``/``prompts`` + and the ``auth``/``deployment`` hints are declared now (open-closed) for the remote connector and other targets to come. >>> spec = IntegrationSpec(name='paths', tools=['os.path:basename']) @@ -46,6 +78,9 @@ class IntegrationSpec: ('paths', ['os.path:basename'], 'local-stdio') >>> IntegrationSpec(name='empty').is_empty() True + >>> draft = IntegrationSpec(name='wx', tool_specs=[ToolSpec(name='get')]) + >>> draft.is_empty(), draft.runnable_refs() + (False, []) """ name: str @@ -54,6 +89,7 @@ class IntegrationSpec: tools: list[str] = field(default_factory=list) # 'module:function' refs (MCP tools) resources: list[str] = field(default_factory=list) # reserved (MCP resources) prompts: list[str] = field(default_factory=list) # reserved (MCP prompts) + tool_specs: list[ToolSpec] = field(default_factory=list) # richer tool descriptors instructions: Optional[str] = None # reserved (SKILL.md procedural knowledge) auth: str = "none" # 'none' | 'env' | 'oauth2.1' (reserved for remote targets) deployment: str = "local-stdio" # 'local-stdio' | 'remote-http' (reserved) @@ -61,8 +97,49 @@ class IntegrationSpec: source: Optional[str] = None # provenance: the skill/module it came from def is_empty(self) -> bool: - """True when there is no connectivity to publish (no tools/resources/prompts).""" - return not (self.tools or self.resources or self.prompts) + """True when there is nothing to publish (no tools/tool_specs/resources/prompts).""" + return not (self.tools or self.tool_specs or self.resources or self.prompts) + + def runnable_refs(self) -> list[str]: + """The importable ``'module:function'`` refs that back *runnable* tools. + + Bare refs in ``tools`` plus the ``handler`` of every *bound* ToolSpec, + de-duplicated preserving order. A draft whose tools are all *proposed* + (unbound) returns ``[]`` — it has nothing a server can actually run yet. + """ + refs = list(self.tools) + for ts in self.tool_specs: + if ts.handler and ts.handler not in refs: + refs.append(ts.handler) + return refs + + def render(self) -> str: + """A terminal-friendly summary of the (possibly draft) integration.""" + lines = [f"IntegrationSpec: {self.name} (v{self.version})"] + if self.description: + lines.append(f" {self.description}") + lines.append(f" deployment: {self.deployment} auth: {self.auth}") + runnable = set(self.runnable_refs()) + if self.tools or self.tool_specs: + lines.append(" tools:") + for ref in self.tools: + lines.append(f" - {ref} [ref]") + for ts in self.tool_specs: + tag = f"bound -> {ts.handler}" if ts.is_bound() else "proposed (no handler)" + lines.append(f" - {ts.name} [{tag}]") + if ts.description: + lines.append(f" {ts.description}") + if self.resources: + lines.append(" resources: " + ", ".join(self.resources)) + if self.prompts: + lines.append(" prompts: " + ", ".join(self.prompts)) + if self.tool_specs and not runnable: + lines.append( + " NOTE: design draft — no tool is bound to importable code. Bind " + "each tool to a 'module:function' handler (or supply refs) before " + "building a runnable .mcpb." + ) + return "\n".join(lines) def integration_spec_from( diff --git a/coact/nl_ingress.py b/coact/nl_ingress.py new file mode 100644 index 0000000..946c79f --- /dev/null +++ b/coact/nl_ingress.py @@ -0,0 +1,282 @@ +"""NL-description → IntegrationSpec — the **opt-in LLM ingress** for PUBLISH. + +:func:`coact.integration.integration_spec_from` is the *mechanical* ingress: it +turns ``'module:function'`` refs / callables / skills into an +:class:`~coact.integration.IntegrationSpec` with **zero** LLM (DECISIONS D10). +This module is the *opt-in* LLM path — it refines a natural-language description +of a desired integration into a **draft** ``IntegrationSpec`` (name, description, +proposed tools with input JSON schemas), routing generation through ``aix`` +(provider-agnostic, per the route-through-aix policy) via ``oa``'s +prompt-as-function machinery. + +D10 is upheld two ways: (1) nothing on a mechanical path imports this module, and +(2) ``oa``/``aix`` are imported **lazily inside** the entry function, so +``import coact`` pulls in neither and a missing backend raises an actionable +``ImportError`` rather than imposing a hard dependency. + +The result is a *draft*: each tool is *proposed* (no importable handler) unless +the description named existing code. Binding proposed tools to real +``module:function`` handlers — or supplying them — is the user's next step before +a runnable ``.mcpb`` can be built (coact writes the design; the user owns the +code, mirroring the D8/D13 scaffold philosophy). +""" + +from __future__ import annotations + +import json +from typing import Any, Callable, Optional + +from coact.integration import IntegrationSpec, ToolSpec +from coact.util import check_requirements, first_balanced_span, to_kebab_case + +LLMPromptFunc = Callable[..., str] + +#: The default integration-authoring prompt. A single ``{description}`` placeholder +#: (no other ``{}`` — the JSON shape is described in prose so ``oa``'s +#: ``str.format`` embodier sees exactly one field). +_INTEGRATION_AUTHORING_TEMPLATE = """\ +You are an expert architect of AI-chatbot integrations (MCP servers). + +Turn the natural-language description below into a structured plan for an +integration that exposes a small set of well-designed tools. + +Respond with ONLY a single JSON object (no prose, no markdown fences). The JSON +object must have exactly these keys: +- "name": a short kebab-case name for the integration (string). +- "description": one sentence summarizing what the integration does (string). +- "tools": an array of tool objects. Each tool object has: + - "name": a snake_case verb_noun tool name (for example get_weather), string. + - "description": what the tool does, when to use it, and key boundaries or + examples. Tool naming and description quality dominate reliability — make + each one precise and unambiguous (string). + - "input_schema": a JSON Schema (draft 2020-12) describing the tool's input — + type object, a properties map, and a required list where appropriate. + - "handler": a "module:function" reference ONLY if the description explicitly + names existing importable code for this tool; otherwise null. +- "resources": an array of short names for readable data resources (or empty). +- "prompts": an array of short names for templated workflows (or empty). + +Design guidance: +- Prefer a few well-named, task-aligned tools over many overlapping ones. +- Keep input schemas minimal and typed; mark genuinely-required fields required. + +Natural-language description of the desired integration: +{description} +""" + +#: Per-target authoring-prompt library. coact owns these templates as the SSOT +#: (one entry per publish target — extend as targets land). ``pyrompt`` is the +#: recommended place to *manage and iterate* on authoring prompts; load one there +#: and pass it via ``prompt_template=`` to override the default used here. +DFLT_AUTHORING_PROMPTS: dict[str, str] = { + "integration": _INTEGRATION_AUTHORING_TEMPLATE, +} + + +def integration_spec_from_description( + description: str, + *, + llm: Any = None, + model: Optional[str] = None, + name: Optional[str] = None, + version: str = "0.1.0", + author: Optional[str] = None, + prompt_template: Optional[str] = None, + infer_tool_schemas: bool = True, +) -> IntegrationSpec: + """Refine a natural-language ``description`` into a **draft** IntegrationSpec. + + The opt-in LLM ingress (DECISIONS D10): generation is routed through ``aix`` + (provider-agnostic) by default, via ``oa``'s prompt-as-function machinery. + + Args: + description: What the integration should do, in plain language. + llm: The LLM backend. ``None`` → ``aix.chat`` (the default, multi-provider); + a ``callable(prompt, **kwargs) -> str`` is used as-is (handy for tests); + a ``str`` is treated as a model name passed to ``aix.chat``. + model: Explicit model id (overrides a model-name ``llm``); ``None`` lets the + backend resolve its own configured default. + name: Force the integration name (else the model proposes one). Kebab-cased. + version: Spec version string. + author: Optional author name recorded on the spec. + prompt_template: Override the authoring prompt (e.g. one managed in + ``pyrompt``). Must contain a single ``{description}`` placeholder. + infer_tool_schemas: When a proposed tool lacks an ``input_schema``, infer + one from its description via ``oa.infer_schema_from_verbal_description`` + (aix-backed). Best-effort — failures leave the schema ``None``. + + Returns: + An :class:`~coact.integration.IntegrationSpec` whose ``tool_specs`` hold the + proposed tools. Tools the description bound to existing code become runnable + refs (``spec.runnable_refs()``); the rest are design drafts to bind later. + + Example (offline, with an injected backend): + + >>> reply = ( + ... '{"name": "wx", "description": "weather",' + ... ' "tools": [{"name": "get_weather", "description": "lookup",' + ... ' "input_schema": {"type": "object", "properties": {"city":' + ... ' {"type": "string"}}}, "handler": null}]}' + ... ) + >>> spec = integration_spec_from_description("a weather tool", llm=lambda p, **k: reply) + >>> spec.name, [t.name for t in spec.tool_specs], spec.runnable_refs() + ('wx', ['get_weather'], []) + """ + if not isinstance(description, str) or not description.strip(): + raise ValueError("description must be a non-empty string") + check_requirements( + {"oa": "oa", "aix": "aix"}, feature="nl-ingress (NL -> IntegrationSpec)" + ) + + from oa.tools import prompt_function # lazy: D10 — no LLM dep on import + + prompt_func, eff_model = _resolve_backend(llm, model) + template = prompt_template or DFLT_AUTHORING_PROMPTS["integration"] + + extract = prompt_function( + template, + prompt_func=prompt_func, + prompt_func_kwargs={"model": eff_model}, + ) + raw = extract(description=description) + data = _parse_json_object(raw) + if data is None: + raise ValueError( + "NL ingress could not parse a JSON IntegrationSpec from the LLM reply. " + "Try a more specific description or a different model (llm=...)." + ) + return _spec_from_extracted( + data, + name=name, + version=version, + author=author, + prompt_func=prompt_func, + model=eff_model, + infer_tool_schemas=infer_tool_schemas, + ) + + +def _resolve_backend( + llm: Any, model: Optional[str] +) -> tuple[LLMPromptFunc, Optional[str]]: + """Resolve ``(prompt_func, model)`` — default to ``aix.chat``; honor injection.""" + if callable(llm): + return llm, model + if isinstance(llm, str): # a model name + return _aix_chat(), (model or llm) + return _aix_chat(), model # None -> aix.chat with the given (or default) model + + +def _aix_chat() -> LLMPromptFunc: + """The ``aix.chat`` callable (lazy import — keeps coact provider-free at import).""" + from aix import chat + + return chat + + +def _spec_from_extracted( + data: dict, + *, + name: Optional[str], + version: str, + author: Optional[str], + prompt_func: LLMPromptFunc, + model: Optional[str], + infer_tool_schemas: bool, +) -> IntegrationSpec: + """Coerce the LLM's extracted dict into a draft :class:`IntegrationSpec`.""" + spec_name = to_kebab_case(name or data.get("name") or "integration") + description = (data.get("description") or "").strip() + + tool_specs: list[ToolSpec] = [] + refs: list[str] = [] + for entry in data.get("tools") or []: + if not isinstance(entry, dict): + continue + tname = (entry.get("name") or "").strip() + if not tname: + continue + tdesc = (entry.get("description") or "").strip() + schema = entry.get("input_schema") + if not isinstance(schema, dict): + schema = None + handler = entry.get("handler") + handler = handler if _looks_like_ref(handler) else None + if schema is None and infer_tool_schemas: + hint = entry.get("input_description") or tdesc + if hint: + schema = _infer_tool_schema(hint, prompt_func=prompt_func, model=model) + tool_specs.append( + ToolSpec( + name=tname, description=tdesc, input_schema=schema, handler=handler + ) + ) + if handler: + refs.append(handler) + + resources = [str(r) for r in (data.get("resources") or []) if r] + prompts = [str(p) for p in (data.get("prompts") or []) if p] + return IntegrationSpec( + name=spec_name, + description=description, + version=version, + tools=refs, + resources=resources, + prompts=prompts, + tool_specs=tool_specs, + author=author, + source="nl-description", + ) + + +def _infer_tool_schema( + input_description: str, *, prompt_func: LLMPromptFunc, model: Optional[str] +) -> Optional[dict]: + """Best-effort input JSON Schema from a tool's prose description (aix-backed). + + Wraps ``oa.infer_schema_from_verbal_description`` (now backend-injectable); + any failure degrades to ``None`` so a flaky per-tool inference never aborts the + whole draft. + """ + try: + from oa.tools import infer_schema_from_verbal_description + + result = infer_schema_from_verbal_description( + input_description, prompt_func=prompt_func, model=model + ) + except Exception: # noqa: BLE001 - inference is optional; degrade gracefully + return None + if not isinstance(result, dict): + return None + props = result.get("properties") + if not isinstance(props, dict): + return None + return {"type": result.get("type", "object"), "properties": props} + + +def _looks_like_ref(value: Any) -> bool: + """True if ``value`` looks like an importable ``'module:function'`` ref.""" + return ( + isinstance(value, str) + and ":" in value + and " " not in value.strip() + and not value.strip().startswith(":") + ) + + +def _parse_json_object(text: Any) -> Optional[dict]: + """Tolerantly pull the first brace-balanced JSON object from an LLM reply. + + Reuses :func:`coact.util.first_balanced_span` (string-aware, depth-balanced) so + a fenced or prose-wrapped reply still parses. Returns ``None`` when no JSON + object is recoverable. + """ + if not isinstance(text, str): + return None + span = first_balanced_span(text, "{", "}") + candidate = span if span is not None else text + try: + obj = json.loads(candidate) + except (ValueError, TypeError): + return None + return obj if isinstance(obj, dict) else None diff --git a/coact/publish_mcpb.py b/coact/publish_mcpb.py index 8681d3f..a2212f9 100644 --- a/coact/publish_mcpb.py +++ b/coact/publish_mcpb.py @@ -77,10 +77,21 @@ def publish_mcpb( if spec.is_empty(): raise ValueError("nothing to publish: the IntegrationSpec carries no tools.") + runnable_refs = spec.runnable_refs() + if not runnable_refs: + proposed = ", ".join(ts.name for ts in spec.tool_specs) or "(none)" + raise ValueError( + f"This IntegrationSpec is a design draft: {len(spec.tool_specs)} " + f"proposed tool(s) [{proposed}], none bound to an importable " + "'module:function' handler. Bind handlers (or pass module:function " + "refs) before building a runnable .mcpb — the draft is still usable as " + "a design artifact (see `coact describe`)." + ) + manifest, warnings = build_manifest( spec, manifest_version=manifest_version, python_command=python_command ) - server_config = {"name": spec.name, "refs": spec.tools} + server_config = {"name": spec.name, "refs": runnable_refs} members = { "manifest.json": json.dumps(manifest, indent=2), "server/main.py": _SERVER_MAIN, @@ -126,8 +137,28 @@ def build_manifest( The server is a Python stdio server launched as ``python server/main.py``; ``${__dirname}`` is resolved by Claude Desktop to the extracted bundle dir. + + Tool metadata is introspected from every *runnable* ref (``module:function`` + in ``tools`` plus bound ToolSpec handlers); any *proposed* (unbound) ToolSpec + is listed by name/description for design visibility, with a warning that it + will not run until bound. """ - tools, warnings = _introspect_tools(spec.tools) + tools, warnings = _introspect_tools(spec.runnable_refs()) + seen = {t["name"] for t in tools} + unbound: list[str] = [] + for ts in spec.tool_specs: + if ts.handler: + continue # already covered via runnable_refs introspection + unbound.append(ts.name) + if ts.name not in seen: + tools.append({"name": ts.name, "description": ts.description}) + seen.add(ts.name) + if unbound: + warnings.append( + f"{len(unbound)} proposed tool(s) have no handler " + f"({', '.join(unbound)}); listed in the manifest for design but they " + "will NOT run until bound to importable module:function handlers." + ) if find_spec("py2mcp") is None: warnings.append( "py2mcp is not importable here; the bundle needs `py2mcp` and " diff --git a/misc/docs/DECISIONS.md b/misc/docs/DECISIONS.md index 49ccafa..232b25e 100644 --- a/misc/docs/DECISIONS.md +++ b/misc/docs/DECISIONS.md @@ -421,3 +421,63 @@ Verified end-to-end: `coact publish os.path:basename --name demo --dest ` writes a valid `.mcpb` (ZIP) with a `manifest_version: "0.3"` manifest (`server.type: python`, `${__dirname}/server/main.py`), docstring-introspected `tools` metadata, and a `py2mcp_config.json` the shim feeds to `py2mcp.serve`. + +## D18 — PUBLISH ingress, part 2: NL-description → IntegrationSpec (the opt-in LLM path) + +D17 shipped the *mechanical* PUBLISH ingress (`integration_spec_from`: refs / +callables / skills → spec, zero LLM). This adds the **second input arm** from the +landscape doc §9.2 — a **natural-language description** → a *draft* +`IntegrationSpec` — as a deliberately separate, opt-in LLM path +(`coact.nl_ingress.integration_spec_from_description`). Decision and rationale: + +- **A separate entry point, not an overload of `integration_spec_from`.** A + `module:function` ref and an NL description are both `str`; routing them through + one function would make the mechanical path's behavior depend on an LLM (a clean + D10 violation, and ambiguous to boot). So `integration_spec_from` stays LLM-free + and `integration_spec_from_description` is the *named* opt-in LLM path. D10 holds + structurally: nothing on a mechanical path imports `nl_ingress`, and `oa`/`aix` + are imported **lazily inside** the entry function (so `import coact` pulls in + neither; a missing backend is an actionable `ImportError`, not a hard dep). + +- **Generation routes through `aix` (provider-agnostic), via `oa`.** Per the + route-through-aix policy, the default backend is `aix.chat` (multi-provider) — + *not* oa's OpenAI default — so the multi-target promise stays honest. The + prompt-as-function machinery is `oa`'s (`prompt_function` for the structured + extraction; the now backend-injectable `oa.infer_schema_from_verbal_description` + for per-tool input-schema inference when a tool lacks one). `oa` was the missing + seam: `infer_schema_from_verbal_description` was hardwired to oa's `chat`; it was + made backend-injectable upstream (first customer = coact, cross-package policy). + The backend is injectable (`llm=` accepts a callable / model-name / `None`), so + the whole path is unit-testable offline with a fake — no provider call. + +- **The result is a *draft*; tools grow a richer descriptor (`ToolSpec`).** The + landscape doc §9.1 always modeled a tool as *(name, description, input schema, + handler ref)*; the D17 P1 simplification (`tools: list[str]` refs) was a + code-path shortcut. NL tools have **no importable handler yet**, so the spec gains + `tool_specs: list[ToolSpec]` (name / description / input_schema / optional + handler) **alongside** the unchanged `tools` refs (non-breaking — refs still + compare equal). A `ToolSpec` *with* a handler is *bound* (its ref joins + `runnable_refs()`); *without*, it is a *proposed* tool — a design draft to bind + before it can run. `is_empty()` now also counts `tool_specs`. + +- **Publishing a draft is honest about runnability (no silent dead bundle).** + `publish_mcpb` builds the server config from `runnable_refs()` (bare refs + + bound-ToolSpec handlers). A pure draft (no runnable ref) **raises** with guidance + rather than writing a `.mcpb` that runs nothing; proposed tools are still listed + in the manifest for design visibility, with a warning that they will not run + until bound. This mirrors the D8/D13 "coact writes the design, the user owns the + code" stance — the draft is a design artifact, not a runnable lie. + +- **Authoring prompts: SSOT in coact, injectable, pyrompt as the iteration home.** + The per-target authoring prompt(s) live in coact (`DFLT_AUTHORING_PROMPTS`, one + entry per target — extend as targets land) so behavior is committed and + reproducible with no hard `pyrompt` dependency. `prompt_template=` lets a caller + inject an alternative (e.g. one *managed/iterated* in `pyrompt`), which is the + right home for prompt curation without duplicating the SSOT into uncommitted + machine-state. (A future `pyrompt`-sync helper can register these for management.) + +- **Packaging.** `coact/nl_ingress.py`; exports `ToolSpec` + + `integration_spec_from_description`; optional extra `coact[nl]` (`oa`, `aix`); CLI + verb `coact describe ""` (renders the draft); skill updated. Offline tests + inject a fake backend (no provider call); the `oa` injection has its own upstream + test + PR. diff --git a/pyproject.toml b/pyproject.toml index d9dd947..80ffd62 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,6 +49,14 @@ mcpb = [ "py2mcp", "fastmcp", ] +# The opt-in NL ingress (`coact describe` / `integration_spec_from_description`) +# routes LLM generation through `aix` (provider-agnostic) via `oa`'s +# prompt-as-function machinery. Both are imported lazily, so they are needed only +# when the NL path is actually used (DECISIONS D10 / D18). +nl = [ + "oa", + "aix", +] litellm = [ "litellm>=1.0", ] diff --git a/tests/test_cli.py b/tests/test_cli.py index 37a63dc..622a1b5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -122,6 +122,34 @@ def test_cli_scaffold_prints_and_writes(tmp_path): assert "Wrote" in wrote and dest.exists() +def test_cli_describe_renders_draft(monkeypatch): + import json + + from coact import nl_ingress + + reply = json.dumps( + { + "name": "wx", + "description": "weather", + "tools": [ + { + "name": "get_weather", + "description": "lookup", + "input_schema": {"type": "object", "properties": {}}, + "handler": None, + } + ], + "resources": [], + "prompts": [], + } + ) + # default backend is aix.chat; stub it so the CLI verb runs offline + monkeypatch.setattr(nl_ingress, "_aix_chat", lambda: (lambda p, **k: reply)) + out = cli.describe("a weather tool") + assert "IntegrationSpec: wx" in out + assert "get_weather" in out and "proposed" in out + + def test_main_wires_every_verb(monkeypatch): # main() registers exactly the documented verbs into argh.dispatch_commands. registered = {} @@ -135,5 +163,5 @@ def fake_dispatch(commands): assert registered["callable"] assert set(registered["names"]) == { "plan", "complete", "emit", "realize", - "diff", "estimate", "inventory", "back", "scaffold", "publish", + "diff", "estimate", "inventory", "back", "scaffold", "publish", "describe", } diff --git a/tests/test_nl_ingress.py b/tests/test_nl_ingress.py new file mode 100644 index 0000000..d452382 --- /dev/null +++ b/tests/test_nl_ingress.py @@ -0,0 +1,216 @@ +"""Tests for the opt-in NL ingress (NL description -> draft IntegrationSpec). + +All offline: the LLM backend is injected as a fake ``callable(prompt, **kw) -> str`` +so no provider is called. They verify the backend is genuinely injectable +(D10/route-through-aix), the draft shape, schema inference fallback, and that a +handler-less draft is rejected by the runnable .mcpb path while a bound one builds. +""" + +import json + +import pytest + +from coact import ( + IntegrationSpec, + ToolSpec, + integration_spec_from_description, + publish, + publish_mcpb, +) + + +def _reply(tools, *, name="wx", description="weather things"): + return json.dumps( + {"name": name, "description": description, "tools": tools, + "resources": [], "prompts": []} + ) + + +def _fixed(reply): + """A fake backend that always returns ``reply`` (records calls).""" + calls = [] + + def fake(prompt, **kwargs): + calls.append((prompt, kwargs)) + return reply + + fake.calls = calls + return fake + + +# --- happy path: proposed tools with inline schemas -------------------------- + + +def test_draft_from_description_with_inline_schemas(): + reply = _reply( + [ + { + "name": "get_weather", + "description": "look up current weather for a city", + "input_schema": { + "type": "object", + "properties": {"city": {"type": "string"}}, + "required": ["city"], + }, + "handler": None, + } + ] + ) + spec = integration_spec_from_description("a weather lookup tool", llm=_fixed(reply)) + assert isinstance(spec, IntegrationSpec) + assert spec.name == "wx" + assert spec.source == "nl-description" + assert [t.name for t in spec.tool_specs] == ["get_weather"] + ts = spec.tool_specs[0] + assert ts.is_bound() is False + assert ts.input_schema["properties"]["city"]["type"] == "string" + # a pure draft has no runnable refs and is not empty + assert spec.runnable_refs() == [] + assert spec.is_empty() is False + + +def test_name_override_is_kebabbed(): + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + spec = integration_spec_from_description("x", llm=_fixed(reply), name="My Cool Connector") + assert spec.name == "my-cool-connector" + + +# --- backend injection (route-through-aix honesty) --------------------------- + + +def test_backend_is_injected_not_openai(): + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + fake = _fixed(reply) + integration_spec_from_description("x", llm=fake) + assert fake.calls, "the injected backend must actually be called" + # the rendered prompt carries the description (the single template placeholder) + assert "x" in fake.calls[0][0] + + +def test_model_string_llm_threads_model(): + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + captured = {} + + def fake(prompt, **kwargs): + captured.update(kwargs) + return reply + + # a str llm is a model name -> threaded as model=... to the backend + integration_spec_from_description("x", llm=fake, model="claude-sonnet-4") + assert captured.get("model") == "claude-sonnet-4" + + +# --- per-tool schema inference fallback -------------------------------------- + + +def test_schema_inference_fallback_invoked_when_missing(): + """A tool without input_schema triggers oa.infer_schema_from_verbal_description.""" + primary = _reply( + [{"name": "t", "description": "does a thing with a count", "handler": None}] + ) + schema_reply = json.dumps( + {"name": "t_in", "properties": {"count": {"type": "integer"}}, "type": "object"} + ) + + def dispatch(prompt, **kwargs): + # the authoring prompt mentions "architect"; the schema prompt mentions "JSON Schema" + return primary if "architect" in prompt else schema_reply + + spec = integration_spec_from_description("x", llm=dispatch, infer_tool_schemas=True) + ts = spec.tool_specs[0] + assert ts.input_schema == {"type": "object", "properties": {"count": {"type": "integer"}}} + + +def test_schema_inference_can_be_disabled(): + primary = _reply([{"name": "t", "description": "d", "handler": None}]) + spec = integration_spec_from_description( + "x", llm=_fixed(primary), infer_tool_schemas=False + ) + assert spec.tool_specs[0].input_schema is None + + +def test_schema_inference_degrades_on_bad_reply(): + """If schema inference returns junk, the tool keeps input_schema=None (no crash).""" + primary = _reply([{"name": "t", "description": "d", "handler": None}]) + + def dispatch(prompt, **kwargs): + return primary if "architect" in prompt else "not json at all" + + spec = integration_spec_from_description("x", llm=dispatch, infer_tool_schemas=True) + assert spec.tool_specs[0].input_schema is None + + +# --- bound handlers become runnable refs -> publishable ---------------------- + + +def test_bound_handler_becomes_runnable_and_publishes(tmp_path): + reply = _reply( + [ + { + "name": "basename", + "description": "basename of a path", + "input_schema": {"type": "object", "properties": {}}, + "handler": "os.path:basename", + } + ] + ) + spec = integration_spec_from_description("expose os.path.basename", llm=_fixed(reply)) + assert spec.runnable_refs() == ["os.path:basename"] + res = publish(spec, name="paths", dest=str(tmp_path)) + assert res.artifact is not None and res.artifact.exists() + + +def test_pure_draft_publish_rejected(): + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + spec = integration_spec_from_description("x", llm=_fixed(reply)) + with pytest.raises(ValueError, match="design draft"): + publish_mcpb(spec) + + +# --- manifest visibility for proposed tools ---------------------------------- + + +def test_proposed_tools_listed_in_manifest_with_warning(tmp_path): + # a bound tool (publishable) plus an unbound proposed tool + spec = IntegrationSpec( + name="mix", + tool_specs=[ + ToolSpec(name="basename", handler="os.path:basename"), + ToolSpec(name="future_tool", description="not built yet"), + ], + ) + res = publish(spec, dest=str(tmp_path), dry_run=True) + assert any("future_tool" in w for w in res.warnings) + + +# --- guards ------------------------------------------------------------------ + + +def test_empty_description_rejected(): + with pytest.raises(ValueError): + integration_spec_from_description(" ", llm=_fixed("{}")) + + +def test_unparseable_reply_rejected(): + with pytest.raises(ValueError, match="could not parse"): + integration_spec_from_description("x", llm=_fixed("sorry, I cannot help")) + + +def test_import_coact_is_provider_free(): + """D10/D18: `import coact` must not pull in oa/aix/litellm (lazy provider deps). + + Run in a subprocess because the test session imports oa/aix elsewhere. + """ + import subprocess + import sys + + code = ( + "import sys, coact; " + "leaked=[m for m in ('oa','aix','litellm','openai') if m in sys.modules]; " + "assert not leaked, leaked; print('ok')" + ) + out = subprocess.run( + [sys.executable, "-c", code], capture_output=True, text=True + ) + assert out.returncode == 0, out.stderr + assert out.stdout.strip() == "ok" From aff289525617ecc3f134c01641736fef99132f51 Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:19:15 +0200 Subject: [PATCH 2/3] harden: review fixes for the NL ingress (9 adversarial-review findings) An adversarial multi-agent review of the P2 diff surfaced 9 confirmed issues, all fixed (offline regression tests added for each): - [high] integration_spec_from dropped a spec's tool_specs/resources/prompts when the spec was passed inside a list -> bound handlers silently lost. Now carried through; the empty-guard counts them too. - [med] non-string LLM field values (number/list/object where a string was asked) crashed _spec_from_extracted with a raw AttributeError/TypeError. Now coerced. - [med] _parse_json_object seized the FIRST balanced {...}; brace-bearing prose before the real JSON defeated it. Now tries whole-text, fenced body, then each top-level balanced span. - [med] NL ingress hard-required aix even when a callable backend was injected. check_requirements is now path-aware (aix only on the default/model-name path). - [low] a bare-string resources/prompts was exploded per character. Now wrapped. - [low] .mcpb draft-guard said "0 proposed tool(s)" for a resources/prompts-only spec. Now a distinct, accurate message. - [low] manifest could advertise a curated name the server would not serve. Keep the bound function's own name/docstring (runtime truth); a curated description only fills an empty one. - [low] str-llm (model-name) branch was untested; README NL snippet implied it was offline. Added tests + an honest README caveat. Full suite: 393 passed, 4 skipped; ruff clean. --- README.md | 5 +- coact/integration.py | 14 +++- coact/nl_ingress.py | 100 ++++++++++++++++++----- coact/publish_mcpb.py | 44 +++++++--- misc/docs/DECISIONS.md | 15 ++++ tests/test_nl_ingress.py | 171 ++++++++++++++++++++++++++++++++++++++- 6 files changed, 313 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index bb89949..04c78dd 100644 --- a/README.md +++ b/README.md @@ -119,8 +119,11 @@ input schemas — routing generation through [`aix`](https://github.com/thorwhal ```python from coact import integration_spec_from_description +# This routes through aix/oa — it makes a real LLM call (needs a configured +# provider). Inject `llm=` to run it offline (e.g. in tests). spec = integration_spec_from_description("expose os.path.basename as a tool") -print(spec.render()) # tools the description bound to code become runnable refs +print(spec.render()) # a DRAFT: a tool becomes a runnable ref only if the model + # binds it to module:function code — else it stays proposed ``` The draft is a **design artifact**: tools without a `module:function` handler are diff --git a/coact/integration.py b/coact/integration.py index d99c606..946afd5 100644 --- a/coact/integration.py +++ b/coact/integration.py @@ -167,6 +167,9 @@ def integration_spec_from( return source refs: list[str] = [] + spec_tool_specs: list[ToolSpec] = [] + spec_resources: list[str] = [] + spec_prompts: list[str] = [] derived_name: Optional[str] = name src_label: Optional[str] = None unrecognized: list[str] = [] @@ -174,7 +177,13 @@ def integration_spec_from( for item in items: if isinstance(item, IntegrationSpec): + # Carry the spec's richer connectivity, not just its bare refs — else a + # draft from `integration_spec_from_description` (whose runnable tools + # live in tool_specs[].handler) would silently lose every handler. refs.extend(item.tools) + spec_tool_specs.extend(item.tool_specs) + spec_resources.extend(item.resources) + spec_prompts.extend(item.prompts) derived_name = derived_name or item.name elif _is_skill_obj(item): # before callable(): a Skill may define __call__ sk_refs, sk_name = _refs_and_name_from_skill(item) @@ -200,7 +209,7 @@ def integration_spec_from( + ". Expected a 'module:function' ref (note the colon), an existing " "skill directory / SKILL.md, or a live callable." ) - if not refs: + if not (refs or spec_tool_specs or spec_resources or spec_prompts): raise ValueError( "No tools found to publish. Provide 'module:function' refs, live " "callables, or a skill carrying a `coact: mcp:` block (module + functions)." @@ -211,6 +220,9 @@ def integration_spec_from( description=description, version=version, tools=refs, + resources=spec_resources, + prompts=spec_prompts, + tool_specs=spec_tool_specs, author=author, source=src_label, ) diff --git a/coact/nl_ingress.py b/coact/nl_ingress.py index 946c79f..95517ff 100644 --- a/coact/nl_ingress.py +++ b/coact/nl_ingress.py @@ -24,6 +24,7 @@ from __future__ import annotations import json +import re from typing import Any, Callable, Optional from coact.integration import IntegrationSpec, ToolSpec @@ -124,9 +125,12 @@ def integration_spec_from_description( """ if not isinstance(description, str) or not description.strip(): raise ValueError("description must be a non-empty string") - check_requirements( - {"oa": "oa", "aix": "aix"}, feature="nl-ingress (NL -> IntegrationSpec)" - ) + # `oa` is used on every path; `aix` only backs the default (None / model-name) + # path — an injected callable needs neither aix installed nor a provider call. + reqs = {"oa": "oa"} + if not callable(llm): + reqs["aix"] = "aix" + check_requirements(reqs, feature="nl-ingress (NL -> IntegrationSpec)") from oa.tools import prompt_function # lazy: D10 — no LLM dep on import @@ -184,26 +188,32 @@ def _spec_from_extracted( model: Optional[str], infer_tool_schemas: bool, ) -> IntegrationSpec: - """Coerce the LLM's extracted dict into a draft :class:`IntegrationSpec`.""" - spec_name = to_kebab_case(name or data.get("name") or "integration") - description = (data.get("description") or "").strip() + """Coerce the LLM's (untrusted) extracted dict into a draft :class:`IntegrationSpec`. + + Every field is coerced defensively: the model may return a number/list/object + where a string was asked for, so ``.strip()``/``to_kebab_case`` never see a + non-string, and a bare-string ``resources``/``prompts`` is wrapped (not + iterated character-by-character). + """ + spec_name = to_kebab_case(name or _as_str(data.get("name")) or "integration") + description = _as_str(data.get("description")).strip() tool_specs: list[ToolSpec] = [] refs: list[str] = [] for entry in data.get("tools") or []: if not isinstance(entry, dict): continue - tname = (entry.get("name") or "").strip() + tname = _as_str(entry.get("name")).strip() if not tname: continue - tdesc = (entry.get("description") or "").strip() + tdesc = _as_str(entry.get("description")).strip() schema = entry.get("input_schema") if not isinstance(schema, dict): schema = None handler = entry.get("handler") handler = handler if _looks_like_ref(handler) else None if schema is None and infer_tool_schemas: - hint = entry.get("input_description") or tdesc + hint = _as_str(entry.get("input_description")) or tdesc if hint: schema = _infer_tool_schema(hint, prompt_func=prompt_func, model=model) tool_specs.append( @@ -214,8 +224,8 @@ def _spec_from_extracted( if handler: refs.append(handler) - resources = [str(r) for r in (data.get("resources") or []) if r] - prompts = [str(p) for p in (data.get("prompts") or []) if p] + resources = _as_str_list(data.get("resources")) + prompts = _as_str_list(data.get("prompts")) return IntegrationSpec( name=spec_name, description=description, @@ -264,19 +274,65 @@ def _looks_like_ref(value: Any) -> bool: ) +def _as_str(value: Any) -> str: + """Coerce an LLM-supplied scalar field to a string (untrusted output may not be).""" + if isinstance(value, str): + return value + return "" if value is None else str(value) + + +def _as_str_list(value: Any) -> list[str]: + """Coerce an LLM-supplied field to a list of non-empty strings. + + A bare string is *wrapped* (not iterated character-by-character); a list/tuple + is element-coerced and emptied of blanks; anything else degrades sensibly. + """ + if value is None: + return [] + if isinstance(value, str): + return [value] if value.strip() else [] + if isinstance(value, (list, tuple)): + return [s for s in (_as_str(v).strip() for v in value) if s] + coerced = _as_str(value).strip() + return [coerced] if coerced else [] + + def _parse_json_object(text: Any) -> Optional[dict]: - """Tolerantly pull the first brace-balanced JSON object from an LLM reply. + """Tolerantly pull a JSON object out of an LLM reply. - Reuses :func:`coact.util.first_balanced_span` (string-aware, depth-balanced) so - a fenced or prose-wrapped reply still parses. Returns ``None`` when no JSON - object is recoverable. + Tries, in order, the whole stripped reply, a fenced ```` ```json … ``` ```` body, + then each top-level brace-balanced ``{…}`` span (string-aware, via + :func:`coact.util.first_balanced_span`) — so a reply whose real JSON is preceded + by brace-bearing prose still parses (a single-span parse would wrongly seize the + first, invalid, fragment). Returns ``None`` when no JSON *object* is recoverable. """ if not isinstance(text, str): return None - span = first_balanced_span(text, "{", "}") - candidate = span if span is not None else text - try: - obj = json.loads(candidate) - except (ValueError, TypeError): - return None - return obj if isinstance(obj, dict) else None + stripped = text.strip() + candidates = [stripped] + fence = re.search(r"```(?:json)?\s*(\{.*?\})\s*```", stripped, re.DOTALL) + if fence: + candidates.append(fence.group(1)) + candidates.extend(_iter_balanced_objects(stripped)) + for candidate in candidates: + try: + obj = json.loads(candidate) + except (ValueError, TypeError): + continue + if isinstance(obj, dict): + return obj + return None + + +def _iter_balanced_objects(s: str): + """Yield each top-level brace-balanced ``{…}`` substring of ``s``, left to right.""" + i = 0 + while i < len(s): + start = s.find("{", i) + if start < 0: + return + span = first_balanced_span(s[start:], "{", "}") + if span is None: + return + yield span + i = start + len(span) diff --git a/coact/publish_mcpb.py b/coact/publish_mcpb.py index a2212f9..b65569e 100644 --- a/coact/publish_mcpb.py +++ b/coact/publish_mcpb.py @@ -79,13 +79,26 @@ def publish_mcpb( runnable_refs = spec.runnable_refs() if not runnable_refs: - proposed = ", ".join(ts.name for ts in spec.tool_specs) or "(none)" + if spec.tool_specs: # proposed tools exist, none bound -> a design draft + proposed = ", ".join(ts.name for ts in spec.tool_specs) + raise ValueError( + f"This IntegrationSpec is a design draft: {len(spec.tool_specs)} " + f"proposed tool(s) [{proposed}], none bound to an importable " + "'module:function' handler. Bind handlers (or pass module:function " + "refs) before building a runnable .mcpb — the draft is still usable " + "as a design artifact (see `coact describe`)." + ) + declared = [] + if spec.resources: + declared.append(f"{len(spec.resources)} resource(s)") + if spec.prompts: + declared.append(f"{len(spec.prompts)} prompt(s)") raise ValueError( - f"This IntegrationSpec is a design draft: {len(spec.tool_specs)} " - f"proposed tool(s) [{proposed}], none bound to an importable " - "'module:function' handler. Bind handlers (or pass module:function " - "refs) before building a runnable .mcpb — the draft is still usable as " - "a design artifact (see `coact describe`)." + "nothing to publish to a .mcpb: this IntegrationSpec declares " + + (", ".join(declared) or "no tools") + + " but no tools. The claude-local-mcpb target consumes tools only " + "(resources/prompts are reserved for future targets) — add " + "'module:function' tool refs (or bound ToolSpecs) to publish." ) manifest, warnings = build_manifest( @@ -139,11 +152,22 @@ def build_manifest( ``${__dirname}`` is resolved by Claude Desktop to the extracted bundle dir. Tool metadata is introspected from every *runnable* ref (``module:function`` - in ``tools`` plus bound ToolSpec handlers); any *proposed* (unbound) ToolSpec - is listed by name/description for design visibility, with a warning that it - will not run until bound. + in ``tools`` plus bound ToolSpec handlers) — the bound function's own + name/docstring are authoritative because they are what ``py2mcp`` actually + serves at runtime (so the manifest never advertises a name the server won't + expose). A bound ToolSpec's curated description only *fills in* an empty + docstring. Any *proposed* (unbound) ToolSpec is listed by name/description for + design visibility, with a warning that it will not run until bound. """ - tools, warnings = _introspect_tools(spec.runnable_refs()) + runnable = spec.runnable_refs() + tools, warnings = _introspect_tools(runnable) + # Fill an empty introspected description from the bound ToolSpec's curated one + # (a fidelity win that can't desync from runtime — names are left untouched). + bound_by_ref = {ts.handler: ts for ts in spec.tool_specs if ts.handler} + for tool, ref in zip(tools, runnable): + ts = bound_by_ref.get(ref) + if ts and ts.description and not tool["description"]: + tool["description"] = ts.description seen = {t["name"] for t in tools} unbound: list[str] = [] for ts in spec.tool_specs: diff --git a/misc/docs/DECISIONS.md b/misc/docs/DECISIONS.md index 232b25e..b934009 100644 --- a/misc/docs/DECISIONS.md +++ b/misc/docs/DECISIONS.md @@ -481,3 +481,18 @@ landscape doc §9.2 — a **natural-language description** → a *draft* verb `coact describe ""` (renders the draft); skill updated. Offline tests inject a fake backend (no provider call); the `oa` injection has its own upstream test + PR. + +- **Review hardening (adversarial multi-agent pass, 9 confirmed findings, all + fixed).** The NL output is *untrusted*, so every extracted field is coerced + (non-string name/description → `str`; a bare-string `resources`/`prompts` is + wrapped, not iterated per-char) and the JSON parse tries the whole reply, a fenced + body, then *each* top-level balanced `{…}` span (brace-bearing prose before the + JSON no longer defeats it). `integration_spec_from` now carries a spec's + `tool_specs`/`resources`/`prompts` through the **list** branch (a draft mixed into + a list previously lost its bound handlers silently). `check_requirements` is + path-aware (an injected callable needs neither `aix` installed nor a provider + call — honoring the "offline when injected" contract). The `.mcpb` draft guard + distinguishes a *design draft* (proposed tools) from a *tools-less* spec + (resources/prompts only) in its message. The manifest keeps the bound function's + own name/docstring (what `py2mcp` actually serves — no manifest↔runtime desync); a + curated ToolSpec description only *fills an empty* one. diff --git a/tests/test_nl_ingress.py b/tests/test_nl_ingress.py index d452382..e7fe7ef 100644 --- a/tests/test_nl_ingress.py +++ b/tests/test_nl_ingress.py @@ -87,7 +87,7 @@ def test_backend_is_injected_not_openai(): assert "x" in fake.calls[0][0] -def test_model_string_llm_threads_model(): +def test_callable_llm_threads_explicit_model(): reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) captured = {} @@ -95,11 +95,38 @@ def fake(prompt, **kwargs): captured.update(kwargs) return reply - # a str llm is a model name -> threaded as model=... to the backend integration_spec_from_description("x", llm=fake, model="claude-sonnet-4") assert captured.get("model") == "claude-sonnet-4" +def test_str_llm_is_treated_as_model_name(monkeypatch): + """A str llm is a model name routed to the default aix.chat backend (D18).""" + from coact import nl_ingress + + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + captured = {} + + def fake_chat(prompt, **kwargs): + captured.update(kwargs) + return reply + + monkeypatch.setattr(nl_ingress, "_aix_chat", lambda: fake_chat) + integration_spec_from_description("x", llm="claude-sonnet-4") + assert captured.get("model") == "claude-sonnet-4" + + +def test_explicit_model_overrides_str_llm(monkeypatch): + from coact import nl_ingress + + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + captured = {} + monkeypatch.setattr( + nl_ingress, "_aix_chat", lambda: (lambda p, **k: captured.update(k) or reply) + ) + integration_spec_from_description("x", llm="model-a", model="model-b") + assert captured.get("model") == "model-b" # explicit model wins over str llm + + # --- per-tool schema inference fallback -------------------------------------- @@ -196,6 +223,146 @@ def test_unparseable_reply_rejected(): integration_spec_from_description("x", llm=_fixed("sorry, I cannot help")) +# --- robustness against untrusted LLM output (review hardening) -------------- + + +def test_non_string_fields_do_not_crash(): + """Numbers/objects where strings were asked for are coerced, not crashed on.""" + reply = json.dumps( + { + "name": 2025, # number where a string was expected + "description": ["a", "list"], + "tools": [ + {"name": 42, "description": {"k": "v"}, "input_schema": {}, "handler": None} + ], + "resources": [], + "prompts": [], + } + ) + spec = integration_spec_from_description("x", llm=_fixed(reply)) + assert spec.name == "2025" # coerced + kebabbed, not a crash + assert spec.tool_specs[0].name == "42" + + +def test_single_string_resources_not_exploded(): + """A bare-string resources/prompts is wrapped, not iterated char-by-char.""" + reply = json.dumps( + { + "name": "wx", + "description": "d", + "tools": [{"name": "t", "description": "d", "input_schema": {}, "handler": None}], + "resources": "the_only_resource", # a string, not a list + "prompts": "the_only_prompt", + } + ) + spec = integration_spec_from_description("x", llm=_fixed(reply)) + assert spec.resources == ["the_only_resource"] + assert spec.prompts == ["the_only_prompt"] + + +def test_parse_recovers_json_after_brace_bearing_prose(): + """A reply whose JSON is preceded by prose containing braces still parses.""" + reply = ( + 'For example {x, y} are inputs. Here is the spec: ' + + _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + ) + spec = integration_spec_from_description("x", llm=_fixed(reply)) + assert spec.name == "wx" and spec.tool_specs[0].name == "t" + + +def test_parse_recovers_fenced_json(): + reply = "```json\n" + _reply( + [{"name": "t", "description": "d", "input_schema": {}, "handler": None}] + ) + "\n```" + spec = integration_spec_from_description("x", llm=_fixed(reply)) + assert spec.name == "wx" + + +def test_injected_callable_does_not_require_aix(monkeypatch): + """With an injected callable backend, aix need not be importable (D18).""" + import builtins + + real_import = builtins.__import__ + + def no_aix(name, *args, **kwargs): + if name == "aix" or name.startswith("aix."): + raise ImportError("aix blocked for test") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", no_aix) + reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) + spec = integration_spec_from_description("x", llm=_fixed(reply)) # must not raise + assert spec.name == "wx" + + +# --- integration_spec_from preserves drafts through the list branch (#5) ------ + + +def test_spec_in_list_preserves_bound_handlers(): + from coact import integration_spec_from + + draft = IntegrationSpec(name="wx", tool_specs=[ToolSpec(name="dn", handler="os.path:dirname")]) + out = integration_spec_from([draft, "os.path:basename"]) + assert "os.path:dirname" in out.runnable_refs() + assert "os.path:basename" in out.runnable_refs() + + +def test_pure_bound_draft_in_list_publishes(tmp_path): + from coact import integration_spec_from + + draft = IntegrationSpec(name="wx", tool_specs=[ToolSpec(name="bn", handler="os.path:basename")]) + out = integration_spec_from([draft]) + assert out.runnable_refs() == ["os.path:basename"] + res = publish(out, dest=str(tmp_path)) + assert res.artifact is not None and res.artifact.exists() + + +# --- manifest fidelity + clearer guards (#4, #7) ----------------------------- + + +def test_bound_toolspec_fills_empty_manifest_description(monkeypatch): + """A bound tool whose docstring is empty gets its manifest desc from the ToolSpec.""" + import importlib + + pm = importlib.import_module("coact.publish_mcpb") + + monkeypatch.setattr( + pm, "_introspect_tools", lambda refs: ([{"name": "basename", "description": ""}], []) + ) + spec = IntegrationSpec( + name="mix", + tool_specs=[ToolSpec(name="x", description="curated", handler="os.path:basename")], + ) + manifest, _ = pm.build_manifest(spec) + tool = next(t for t in manifest["tools"] if t["name"] == "basename") + assert tool["description"] == "curated" # empty introspected desc filled in + + +def test_bound_toolspec_does_not_clobber_introspected_description(monkeypatch): + """Runtime truth (the function's own docstring) wins — never desync manifest vs server.""" + import importlib + + pm = importlib.import_module("coact.publish_mcpb") + + monkeypatch.setattr( + pm, + "_introspect_tools", + lambda refs: ([{"name": "basename", "description": "real docstring"}], []), + ) + spec = IntegrationSpec( + name="mix", + tool_specs=[ToolSpec(name="x", description="curated", handler="os.path:basename")], + ) + manifest, _ = pm.build_manifest(spec) + tool = next(t for t in manifest["tools"] if t["name"] == "basename") + assert tool["description"] == "real docstring" + + +def test_resources_only_spec_rejected_with_clear_message(): + with pytest.raises(ValueError, match="resource"): + publish_mcpb(IntegrationSpec(name="r", resources=["data1"])) + + def test_import_coact_is_provider_free(): """D10/D18: `import coact` must not pull in oa/aix/litellm (lazy provider deps). From f86497639c85674083deeaea6e1bd45b173ce1eb Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:25:14 +0200 Subject: [PATCH 3/3] test(ci): skip oa-dependent NL tests in bare CI; keep mechanical regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI does not install the optional `coact[nl]` extra, so the NL ingress path (which genuinely needs `oa` as its prompt orchestrator) cannot run there. Match the project's established posture (litellm/langgraph/crewai backend tests skip in bare CI, run in dev): - test_nl_ingress.py guards the whole module with pytest.importorskip("oa") (aix is never imported — backends are injected/monkeypatched). - the oa-FREE mechanical regressions moved to test_publish.py so they still run in CI: the high-severity list-branch handler-preservation fix (#5), manifest fidelity (#4), the resources-only guard message (#7), proposed-tool warning, and the D10 import-isolation check. - the `coact describe` CLI test guards with importorskip("oa"). Dev: 393 passed, 4 skipped. Bare-CI simulation (oa/aix blocked): the affected files run with 0 failures (oa-dependent tests skip, oa-free ones pass). --- tests/test_cli.py | 1 + tests/test_nl_ingress.py | 113 +++------------------------------------ tests/test_publish.py | 96 +++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 105 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 622a1b5..82b9b95 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -125,6 +125,7 @@ def test_cli_scaffold_prints_and_writes(tmp_path): def test_cli_describe_renders_draft(monkeypatch): import json + pytest.importorskip("oa") # the describe verb uses oa's prompt machinery from coact import nl_ingress reply = json.dumps( diff --git a/tests/test_nl_ingress.py b/tests/test_nl_ingress.py index e7fe7ef..94c01ec 100644 --- a/tests/test_nl_ingress.py +++ b/tests/test_nl_ingress.py @@ -10,7 +10,14 @@ import pytest -from coact import ( +# The NL ingress path uses oa's prompt-as-function machinery — installed in dev, +# skipped in bare CI (mirrors the litellm/langgraph/crewai backend tests). The +# oa-FREE mechanical regressions (list-branch preservation, manifest fidelity, +# resources-only guard, D10 import isolation) live in test_publish.py so they run +# in CI regardless. aix is never imported here (backends are injected/monkeypatched). +pytest.importorskip("oa") + +from coact import ( # noqa: E402 - after importorskip by design IntegrationSpec, ToolSpec, integration_spec_from_description, @@ -194,22 +201,6 @@ def test_pure_draft_publish_rejected(): publish_mcpb(spec) -# --- manifest visibility for proposed tools ---------------------------------- - - -def test_proposed_tools_listed_in_manifest_with_warning(tmp_path): - # a bound tool (publishable) plus an unbound proposed tool - spec = IntegrationSpec( - name="mix", - tool_specs=[ - ToolSpec(name="basename", handler="os.path:basename"), - ToolSpec(name="future_tool", description="not built yet"), - ], - ) - res = publish(spec, dest=str(tmp_path), dry_run=True) - assert any("future_tool" in w for w in res.warnings) - - # --- guards ------------------------------------------------------------------ @@ -293,91 +284,3 @@ def no_aix(name, *args, **kwargs): reply = _reply([{"name": "t", "description": "d", "input_schema": {}, "handler": None}]) spec = integration_spec_from_description("x", llm=_fixed(reply)) # must not raise assert spec.name == "wx" - - -# --- integration_spec_from preserves drafts through the list branch (#5) ------ - - -def test_spec_in_list_preserves_bound_handlers(): - from coact import integration_spec_from - - draft = IntegrationSpec(name="wx", tool_specs=[ToolSpec(name="dn", handler="os.path:dirname")]) - out = integration_spec_from([draft, "os.path:basename"]) - assert "os.path:dirname" in out.runnable_refs() - assert "os.path:basename" in out.runnable_refs() - - -def test_pure_bound_draft_in_list_publishes(tmp_path): - from coact import integration_spec_from - - draft = IntegrationSpec(name="wx", tool_specs=[ToolSpec(name="bn", handler="os.path:basename")]) - out = integration_spec_from([draft]) - assert out.runnable_refs() == ["os.path:basename"] - res = publish(out, dest=str(tmp_path)) - assert res.artifact is not None and res.artifact.exists() - - -# --- manifest fidelity + clearer guards (#4, #7) ----------------------------- - - -def test_bound_toolspec_fills_empty_manifest_description(monkeypatch): - """A bound tool whose docstring is empty gets its manifest desc from the ToolSpec.""" - import importlib - - pm = importlib.import_module("coact.publish_mcpb") - - monkeypatch.setattr( - pm, "_introspect_tools", lambda refs: ([{"name": "basename", "description": ""}], []) - ) - spec = IntegrationSpec( - name="mix", - tool_specs=[ToolSpec(name="x", description="curated", handler="os.path:basename")], - ) - manifest, _ = pm.build_manifest(spec) - tool = next(t for t in manifest["tools"] if t["name"] == "basename") - assert tool["description"] == "curated" # empty introspected desc filled in - - -def test_bound_toolspec_does_not_clobber_introspected_description(monkeypatch): - """Runtime truth (the function's own docstring) wins — never desync manifest vs server.""" - import importlib - - pm = importlib.import_module("coact.publish_mcpb") - - monkeypatch.setattr( - pm, - "_introspect_tools", - lambda refs: ([{"name": "basename", "description": "real docstring"}], []), - ) - spec = IntegrationSpec( - name="mix", - tool_specs=[ToolSpec(name="x", description="curated", handler="os.path:basename")], - ) - manifest, _ = pm.build_manifest(spec) - tool = next(t for t in manifest["tools"] if t["name"] == "basename") - assert tool["description"] == "real docstring" - - -def test_resources_only_spec_rejected_with_clear_message(): - with pytest.raises(ValueError, match="resource"): - publish_mcpb(IntegrationSpec(name="r", resources=["data1"])) - - -def test_import_coact_is_provider_free(): - """D10/D18: `import coact` must not pull in oa/aix/litellm (lazy provider deps). - - Run in a subprocess because the test session imports oa/aix elsewhere. - """ - import subprocess - import sys - - code = ( - "import sys, coact; " - "leaked=[m for m in ('oa','aix','litellm','openai') if m in sys.modules]; " - "assert not leaked, leaked; print('ok')" - ) - out = subprocess.run( - [sys.executable, "-c", code], capture_output=True, text=True - ) - assert out.returncode == 0, out.stderr - assert out.stdout.strip() == "ok" diff --git a/tests/test_publish.py b/tests/test_publish.py index de35dcf..3141978 100644 --- a/tests/test_publish.py +++ b/tests/test_publish.py @@ -7,6 +7,7 @@ from coact import ( IntegrationSpec, + ToolSpec, integration_spec_from, publish, publish_mcpb, @@ -168,3 +169,98 @@ def test_duplicate_tool_names_warn(tmp_path): dry_run=True, ) assert any("duplicate tool name" in w for w in res.warnings) + + +# --- ToolSpec / draft mechanics (oa-free; run in CI) ------------------------- +# These cover the IntegrationSpec/publish-axis half of the NL ingress without +# needing the oa backend, so they run in bare CI (the oa-dependent ingress tests +# live in test_nl_ingress.py under importorskip("oa")). + + +def test_proposed_tools_listed_in_manifest_with_warning(tmp_path): + # a bound tool (publishable) plus an unbound proposed tool + spec = IntegrationSpec( + name="mix", + tool_specs=[ + ToolSpec(name="basename", handler="os.path:basename"), + ToolSpec(name="future_tool", description="not built yet"), + ], + ) + res = publish(spec, dest=str(tmp_path), dry_run=True) + assert any("future_tool" in w for w in res.warnings) + + +def test_spec_in_list_preserves_bound_handlers(): + # #5 (high): a draft mixed into a list must not lose its bound handlers + draft = IntegrationSpec( + name="wx", tool_specs=[ToolSpec(name="dn", handler="os.path:dirname")] + ) + out = integration_spec_from([draft, "os.path:basename"]) + assert "os.path:dirname" in out.runnable_refs() + assert "os.path:basename" in out.runnable_refs() + + +def test_pure_bound_draft_in_list_publishes(tmp_path): + draft = IntegrationSpec( + name="wx", tool_specs=[ToolSpec(name="bn", handler="os.path:basename")] + ) + out = integration_spec_from([draft]) + assert out.runnable_refs() == ["os.path:basename"] + res = publish(out, dest=str(tmp_path)) + assert res.artifact is not None and res.artifact.exists() + + +def test_bound_toolspec_fills_empty_manifest_description(monkeypatch): + """A bound tool whose docstring is empty gets its manifest desc from the ToolSpec.""" + import importlib + + pm = importlib.import_module("coact.publish_mcpb") + monkeypatch.setattr( + pm, "_introspect_tools", lambda refs: ([{"name": "basename", "description": ""}], []) + ) + spec = IntegrationSpec( + name="mix", + tool_specs=[ToolSpec(name="x", description="curated", handler="os.path:basename")], + ) + manifest, _ = pm.build_manifest(spec) + tool = next(t for t in manifest["tools"] if t["name"] == "basename") + assert tool["description"] == "curated" # empty introspected desc filled in + + +def test_bound_toolspec_does_not_clobber_introspected_description(monkeypatch): + """Runtime truth (the function's own docstring) wins — never desync manifest vs server.""" + import importlib + + pm = importlib.import_module("coact.publish_mcpb") + monkeypatch.setattr( + pm, + "_introspect_tools", + lambda refs: ([{"name": "basename", "description": "real docstring"}], []), + ) + spec = IntegrationSpec( + name="mix", + tool_specs=[ToolSpec(name="x", description="curated", handler="os.path:basename")], + ) + manifest, _ = pm.build_manifest(spec) + tool = next(t for t in manifest["tools"] if t["name"] == "basename") + assert tool["description"] == "real docstring" + + +def test_resources_only_spec_rejected_with_clear_message(): + with pytest.raises(ValueError, match="resource"): + publish_mcpb(IntegrationSpec(name="r", resources=["data1"])) + + +def test_import_coact_is_provider_free(): + """D10/D18: `import coact` must not pull in oa/aix/litellm (lazy provider deps).""" + import subprocess + import sys + + code = ( + "import sys, coact; " + "leaked=[m for m in ('oa','aix','litellm','openai') if m in sys.modules]; " + "assert not leaked, leaked; print('ok')" + ) + out = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True) + assert out.returncode == 0, out.stderr + assert out.stdout.strip() == "ok"