From 3810bd955169356bc7a454578da53a72e909a8d2 Mon Sep 17 00:00:00 2001 From: Jeremy Bedient Date: Tue, 27 Oct 2026 10:24:07 -0400 Subject: [PATCH] Model a Call LLM step's MCP server and run-as account Kizen's 2026-10-26 release gave every Call LLM step mcp_server, run_tools_as, max_sequential_turns and tool_failure_policy. The CLI refused the unread keys, so show, diff, steps, activate, deactivate, move and roundtrip all stopped on any automation with such a step. Specs can now name the server by api_name and the account by name, resolved against the step's own pickers, and dry-run refuses one without the other, as Kizen does. The builder always sends all four keys: a PUT that omits both ids keeps the server and clears the account, storing a step Kizen's own validator then rejects. A spec without them therefore detaches a server attached in the UI, so update's preview names each step that loses its server or account. --- CHANGELOG.md | 15 + src/kizen_builder/api/automations.py | 15 + src/kizen_builder/docs/specs/automation.md | 37 ++- .../models/spec/automations_actions_code.py | 39 +++ .../tools/planners/automations.py | 135 +++++++- tests/drift/test_roundtrip_automations.py | 67 ++++ .../automations/call_llm_mcp.raw.json | 161 ++++++++++ tests/test_automation_payloads.py | 289 ++++++++++++++++++ tests/test_translate.py | 29 ++ 9 files changed, 785 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/automations/call_llm_mcp.raw.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b9bb40..2b10eee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -253,6 +253,13 @@ called out explicitly under **Changed** or **Removed**. ### Added +- **Automation specs can give a Call LLM step an MCP server's tools:** + `mcp_server` (api_name or UUID), `run_tools_as` (a service account's name + or UUID), `max_sequential_turns` and `tool_failure_policy`. Set the server + and the account together; `--dry-run` refuses one without the other. A + spec that leaves them out gets no server, so `update` detaches one attached + in the UI. `update --dry-run` names those steps (`detaches_mcp_server=`), + and `diff` shows the change. - **`activities duplicate `** copies an activity type, and **`dashboards duplicate `** copies a dashboard with its dashlets. `--name` names the copy, and `dashboards duplicate` also takes @@ -495,6 +502,14 @@ called out explicitly under **Changed** or **Removed**. ### Fixed +- **Automations with an AI (Call LLM) step can be shown, diffed and edited + again.** Since Kizen's 2026-10-26 release every such step carries two + settings the CLI refused, so `show`, `diff`, `steps`, `activate`, + `deactivate`, `move` and `roundtrip` stopped on any automation with one. +- **`automations update` from a spec no longer leaves an MCP-attached Call + LLM step without its run-as account.** Kizen kept the server and dropped + the account when a step's spec left both out, storing a step it then + refuses to save. - **`forms`/`surveys update` and `set-ui` no longer delete fields.** Kizen deletes every field that isn't placed on a page whenever a form is saved, so updating a form with no layout yet emptied it, and a partial `set-ui` diff --git a/src/kizen_builder/api/automations.py b/src/kizen_builder/api/automations.py index dfe2b09..202c3e4 100644 --- a/src/kizen_builder/api/automations.py +++ b/src/kizen_builder/api/automations.py @@ -55,6 +55,21 @@ def get_metadata(client: KizenClient) -> dict[str, Any]: return client.get("/api/automation2/automations/metadata") +def list_mcp_step_options(client: KizenClient) -> list[dict[str, Any]]: + """GET /api/mcp-servers/step-options: the MCP servers a `call_llm` step + can attach, as its picker lists them (`{id, name, api_name, is_enabled, + deleted, tool_count}`). Unpaginated; the server ignores `search`.""" + return client.get("/api/mcp-servers/step-options") + + +def list_run_tools_as_options(client: KizenClient) -> list[dict[str, Any]]: + """GET /api/automation2/automations/run-tools-as-options: the service + accounts a `call_llm` step can run its MCP tools as (`{id, name}`). + Narrower than the service-account typeahead: a plugin's own service + account isn't listed, and the write rejects it.""" + return client.get("/api/automation2/automations/run-tools-as-options") + + def start_automation( client: KizenClient, api_name: str, diff --git a/src/kizen_builder/docs/specs/automation.md b/src/kizen_builder/docs/specs/automation.md index 0ac497c..fcf19b8 100644 --- a/src/kizen_builder/docs/specs/automation.md +++ b/src/kizen_builder/docs/specs/automation.md @@ -174,7 +174,7 @@ to wait for a reply. Its `wait_type` is `never`, `delay` or | `go_to_automation_step` | `action_go_to_automation_step` | jump to another step (`step_key`) | | `code_step` | `action_code_step` | run Python (`script`, `inputs`, `outputs`, `secrets`) | | `plugin_code_step` | `action_plugin_code_step` | run a step that an installed plugin ships | -| `call_llm` | `action_call_llm` | prompt an LLM, write to `destinations` | +| `call_llm` | `action_call_llm` | prompt an LLM, optionally with an MCP server's tools, write to `destinations` | | `file_content_extraction` | `action_file_content_extraction` | extract from a file field on the record, a related record, or in a variable (`input_value_source`) | | `audio_transcription` | `action_audio_transcription` | transcribe an audio field on the record, a related record, or in a variable (`input_value_source`) | | `change_field_value` | `action_change_field_value` | set/clear a field | @@ -538,6 +538,41 @@ still runs fine via the raw API, but its rich-text prompt editor renders blank in the builder UI without this. Pass `html_prompt` explicitly to override the derivation. +**Tools (MCP server)** — a `call_llm` step can let the model call an MCP +server's tools. Confirmed live 2026-10-27: + +| Spec key | Wire key | Value | +|---|---|---| +| `mcp_server` | `mcp_server_id` | the server's `api_name` or UUID | +| `run_tools_as` | `run_tools_as_id` | the service account's `name` or UUID | +| `max_sequential_turns` | same | 1–32767, default 25 | +| `tool_failure_policy` | same | `never_fail`, the only value Kizen accepts | + +- Names resolve against the step's own pickers: + `GET /api/mcp-servers/step-options` for servers and + `GET /api/automation2/automations/run-tools-as-options` for accounts. The + second leaves out a plugin's own service account, and a write naming that + account is a 400. A service account resolves by `name` because a user-made + one has no `api_name`. A name two accounts share is an error; give the UUID. + The `*_id` spellings take the same values. +- Set both `mcp_server` and `run_tools_as`, or neither. Kizen 400s on either + alone, and `--dry-run` refuses it first. +- A model without tool calling 400s with "The selected model does not support + tool calling". The model catalog has no flag for it, so the server's 400 is + the only check. +- Reads expand both references (`mcp_server {id, name, api_name, …}`, + `run_tools_as {id, name}`), and every `call_llm` step reads + `max_sequential_turns` and `tool_failure_policy`, server or not. +- The planner always sends all four keys. A step that omits them gets no + server, no account, 25 turns and `never_fail`, so `update` from a spec + without them **detaches a server attached in the UI**. `update --dry-run` + names those steps in its preview: `detaches_mcp_server=[, …]`, + and `clears_run_tools_as=[…]` for a run-as account that goes. Steps pair + as `diff` pairs them, and `diff` shows each change as + `mcp_server_id: '' → None` and `run_tools_as_id: '' → None`. + Kizen itself, given a PUT that omits both ids, keeps the server and clears + the account, which leaves a step its own validator rejects. + ### Variable-comparison condition step A `condition` step's `filter_config` normally uses the record-field filter diff --git a/src/kizen_builder/models/spec/automations_actions_code.py b/src/kizen_builder/models/spec/automations_actions_code.py index bdfc7ac..e49086b 100644 --- a/src/kizen_builder/models/spec/automations_actions_code.py +++ b/src/kizen_builder/models/spec/automations_actions_code.py @@ -124,6 +124,45 @@ class ActionLlmCallConfig(BaseModel): "audio_transcription, and a condition step's llm_decision block." ), ) + mcp_server: str | dict[str, Any] | None = Field( + default=None, + description=( + "The MCP server whose tools the model may call: its api_name or " + "UUID, from the Call LLM step's own server picker " + "(GET /api/mcp-servers/step-options). Needs run_tools_as. Omitted " + "means no server, so an update detaches one set in the UI." + ), + ) + mcp_server_id: str | None = Field( + default=None, + description="The MCP server's UUID (the wire spelling of mcp_server).", + ) + run_tools_as: str | dict[str, Any] | None = Field( + default=None, + description=( + "The service account the MCP server's tools run as: its name or " + "UUID, from the step's run-as picker " + "(GET /api/automation2/automations/run-tools-as-options), which " + "leaves out a plugin's own service account. Needs mcp_server." + ), + ) + run_tools_as_id: str | None = Field( + default=None, + description="The service account's UUID (the wire spelling of run_tools_as).", + ) + max_sequential_turns: int | None = Field( + default=None, + ge=1, + le=32767, + description="How many model turns the step may take in a row. Default 25.", + ) + tool_failure_policy: Literal["never_fail"] | None = Field( + default=None, + description=( + "What a failed tool call does to the step. 'never_fail' (the " + "default) is the only value Kizen accepts." + ), + ) class ActionAudioTranscriptionConfig(BaseModel): diff --git a/src/kizen_builder/tools/planners/automations.py b/src/kizen_builder/tools/planners/automations.py index 713e85d..be5730b 100644 --- a/src/kizen_builder/tools/planners/automations.py +++ b/src/kizen_builder/tools/planners/automations.py @@ -83,6 +83,52 @@ def __init__(self) -> None: # rather than a single value shape. self._objects_by_api: dict[str, Any] = {} self._automations_by_api: dict[str, dict[str, Any]] | None = None + self._step_pickers: dict[str, list[dict[str, Any]]] = {} + + def mcp_server_uuid(self, api_name: str) -> str: + """UUID of an MCP server a call_llm step can attach, by api_name.""" + from kizen_builder.api.automations import list_mcp_step_options + + options = self._step_picker("mcp_servers", list_mcp_step_options) + match = next((o for o in options if o.get("api_name") == api_name), None) + if match is None: + available = ", ".join(str(o.get("api_name")) for o in options) + raise PlanError( + f"MCP server '{api_name}' isn't one a Call LLM step can attach " + f"in '{self.env}'. Available: {available or '(none)'}" + ) + return match["id"] + + def service_account_uuid(self, name: str) -> str: + """UUID of a service account a call_llm step can run its tools as, + by name: a user-made account has no api_name.""" + from kizen_builder.api.automations import list_run_tools_as_options + + options = self._step_picker("run_tools_as", list_run_tools_as_options) + matches = [o for o in options if o.get("name") == name] + if not matches: + available = ", ".join(str(o.get("name")) for o in options) + raise PlanError( + f"service account '{name}' can't run a Call LLM step's tools " + f"in '{self.env}'. Available: {available or '(none)'}" + ) + if len(matches) > 1: + ids = ", ".join(m["id"] for m in matches) + raise PlanError( + f"service account name '{name}' matches {len(matches)} accounts " + f"in '{self.env}': {ids}. Use the id instead." + ) + return matches[0]["id"] + + def _step_picker( + self, name: str, fetch: Callable[[Any], list[dict[str, Any]]] + ) -> list[dict[str, Any]]: + if name not in self._step_pickers: + from kizen_builder.api.client import KizenClient + + with KizenClient(load_env_config()) as client: + self._step_pickers[name] = fetch(client) + return self._step_pickers[name] def object_uuid(self, api_name: str) -> str: return self.object_data(api_name)["id"] @@ -285,6 +331,7 @@ def plan_update_automation(automation: dict[str, Any] | AutomationDef) -> Plan: payload = _merge_server_state(_build_automation_payload(auto, ctx), current) preview = _automation_preview(env, auto, live_active=live_active) preview["current_revision"] = current.get("revision") + preview.update(_llm_tools_dropped(current, payload)) return Plan.build( env=env, summary=f"Update automation '{auto.api_name}'", @@ -602,6 +649,31 @@ def _fan_out_warnings(auto: AutomationDef) -> dict[str, Any]: return {"warnings": warnings} if warnings else {} +def _llm_tools_dropped( + current: dict[str, Any], payload: dict[str, Any] +) -> dict[str, list[str]]: + """Call LLM steps whose live MCP server or run-as account the update + clears, by spec step key. A spec that omits the keys sends null, so an + `update` detaches a server attached in the UI. Steps pair as `diff` + pairs them: by id, then by position.""" + from kizen_builder.translate import _wire_pairs + + live_steps = sorted(current.get("steps") or [], key=lambda s: s.get("order") or 0) + dropped: dict[str, list[str]] = {} + for live, built in _wire_pairs(live_steps, payload.get("steps") or []): + if live is None or built is None or built.get("type") != "call_llm": + continue + was = live.get("action_call_llm") or {} + now = built.get("action_call_llm") or {} + for read_key, wire_key, cell in ( + ("mcp_server", "mcp_server_id", "detaches_mcp_server"), + ("run_tools_as", "run_tools_as_id", "clears_run_tools_as"), + ): + if was.get(read_key) and now.get(wire_key) is None: + dropped.setdefault(cell, []).append(built["key"]) + return dropped + + # --------------------------------------------------------------------------- # Top-level payload builder # --------------------------------------------------------------------------- @@ -1438,6 +1510,24 @@ def _built_block_problem( return _scalar_value_resolution_problem(key, block, auto) if step_type == "assign_team_member": return _assign_team_member_problem(key, block, auto, ctx) + if step_type == "call_llm": + return _llm_tools_pairing_problem(key, block) + return None + + +def _llm_tools_pairing_problem(key: str, block: dict[str, Any]) -> str | None: + """The write rejects a call_llm step with only one of the pair.""" + server, account = block.get("mcp_server_id"), block.get("run_tools_as_id") + if server and not account: + return ( + f"step '{key}': mcp_server is set without run_tools_as. Kizen runs " + "an MCP server's tools as a service account, so set both or neither." + ) + if account and not server: + return ( + f"step '{key}': run_tools_as is set without mcp_server. " + "Set both or neither." + ) return None @@ -2255,6 +2345,12 @@ def _normalize_variable(v: Any) -> Any: "destinations", "is_advanced", "merge_field_validation", + "mcp_server", + "mcp_server_id", + "run_tools_as", + "run_tools_as_id", + "max_sequential_turns", + "tool_failure_policy", ) def _step_call_llm( block: dict[str, Any], auto: AutomationDef, ctx: LiveContext @@ -2289,7 +2385,44 @@ def _step_call_llm( out["is_advanced"] = block["is_advanced"] if block.get("merge_field_validation"): out["merge_field_validation"] = block["merge_field_validation"] - return {k: v for k, v in out.items() if v is not None} + out = {k: v for k, v in out.items() if v is not None} + # Always sent, null included: a PUT that omits both ids keeps the server + # and clears the account, a pairing the write itself rejects. + out["mcp_server_id"] = _tool_ref_uuid( + "mcp_server", + block.get("mcp_server_id"), + block.get("mcp_server"), + ctx.mcp_server_uuid, + ) + out["run_tools_as_id"] = _tool_ref_uuid( + "run_tools_as", + block.get("run_tools_as_id"), + block.get("run_tools_as"), + ctx.service_account_uuid, + ) + turns = block.get("max_sequential_turns") + out["max_sequential_turns"] = 25 if turns is None else turns + out["tool_failure_policy"] = block.get("tool_failure_policy") or "never_fail" + return out + + +def _tool_ref_uuid( + key: str, id_value: Any, value: Any, resolve: Callable[[str], str] +) -> str | None: + """A call_llm tool reference as the bare UUID the wire takes. A read's + expanded object yields its id and a UUID passes through, so translate + never reaches ``resolve``; any other string is a name to look up.""" + resolved: set[str] = set() + for v in (id_value, value): + if isinstance(v, dict): + v = v.get("id") + elif v is not None and not is_uuid(v): + v = resolve(v) + if v is not None: + resolved.add(v) + if len(resolved) > 1: + raise PlanError(f"set {key} or {key}_id, not both") + return next(iter(resolved), None) def _normalize_destinations(destinations: list[dict[str, Any]]) -> list[dict[str, Any]]: diff --git a/tests/drift/test_roundtrip_automations.py b/tests/drift/test_roundtrip_automations.py index 60e4182..1ea2116 100644 --- a/tests/drift/test_roundtrip_automations.py +++ b/tests/drift/test_roundtrip_automations.py @@ -2067,6 +2067,72 @@ def test_extraction_input_sources_put_back_unchanged( assert list(semantic_diff(before, after)) == [] +@pytest.fixture(scope="session") +def drift_ai_mcp_step( + drift_client, scratch, drift_object, drift_step_fields, drift_llm_models +) -> dict[str, Any]: + """A `call_llm` step with an MCP server and a run-as account, named the + way a spec names them: the first row of each of the step's own pickers. + Skips when either picker is empty. Stored config only, as above.""" + from kizen_builder.api import automations as auto_api + + servers = auto_api.list_mcp_step_options(drift_client) + accounts = auto_api.list_run_tools_as_options(drift_client) + if not servers or not accounts: + pytest.skip( + "the drift environment has no MCP server or no service account a " + "Call LLM step can use" + ) + spec = _base_spec("ai_mcp", drift_object) + spec["steps"] = [ + { + "key": "llm_tools", + "step_type": "call_llm", + "order": 0, + "parent_key": None, + "action_call_llm": _llm_block( + drift_llm_models["call"], + prompt="Summarize this record using the available tools.", + destinations=[{"field": drift_step_fields["longtext"]["id"]}], + mcp_server=servers[0]["api_name"], + run_tools_as=accounts[0]["id"], + max_sequential_turns=10, + ), + }, + ] + record = _create_automation(drift_client, scratch, spec) + return {**record, "server": servers[0], "account": accounts[0]} + + +def test_call_llm_mcp_server_and_account_read_back(drift_ai_mcp_step): + """The wire takes `mcp_server_id`/`run_tools_as_id`; the read expands + both to objects. A spec names the server by api_name.""" + (live,) = _live_steps_by_type(drift_ai_mcp_step)["call_llm"] + block = live["action_call_llm"] + assert block["mcp_server"]["id"] == drift_ai_mcp_step["server"]["id"] + assert block["run_tools_as"]["id"] == drift_ai_mcp_step["account"]["id"] + assert block["max_sequential_turns"] == 10 + assert block["tool_failure_policy"] == "never_fail" + + +def test_call_llm_mcp_step_puts_back_unchanged(drift_client, drift_ai_mcp_step): + """What every read-modify-write verb does: translate, PUT, re-read.""" + from kizen_builder.api import automations as auto_api + from kizen_builder.translate import live_to_payload, semantic_diff, validate_payload + + before = drift_ai_mcp_step["live"] + payload = live_to_payload(before) + assert validate_payload(payload) == [] + auto_api.update_automation( + drift_client, + drift_ai_mcp_step["uuid"], + payload, + last_revision=payload.get("last_revision"), + ) + after = auto_api.get_automation(drift_client, drift_ai_mcp_step["uuid"]) + assert list(semantic_diff(before, after)) == [] + + def test_input_field_reads_back_mirrored_as_a_context_source( drift_ai_steps, drift_step_fields ): @@ -2485,6 +2551,7 @@ def test_start_and_wait_streams_a_live_run_without_crashing( "drift_related_steps", "drift_messaging", "drift_ai_steps", + "drift_ai_mcp_step", "drift_extraction_sources", "drift_plugin_steps", "drift_activity_steps", diff --git a/tests/fixtures/automations/call_llm_mcp.raw.json b/tests/fixtures/automations/call_llm_mcp.raw.json new file mode 100644 index 0000000..88405b1 --- /dev/null +++ b/tests/fixtures/automations/call_llm_mcp.raw.json @@ -0,0 +1,161 @@ +{ + "id": "9fa618d0-380c-448d-9ed4-7a0d89847e2c", + "name": "Call LLM MCP Tools", + "api_name": "call_llm_mcp", + "ai_description": null, + "active": false, + "type": "record_based", + "custom_object": { + "id": "8c3f0149-6915-4370-940f-2e61848d6842", + "name": "tickets", + "description": null, + "entity_name": "ticket", + "object_name": "ticket", + "object_class": "custom_objects", + "object_type": "pipeline", + "default_on_activities": true, + "deleted": false + }, + "user_description": "", + "folder": { + "id": "44b300c0-ce2c-4c49-a427-44a25f85bfad", + "name": "" + }, + "triggers": [ + { + "id": "f2aa539d-a9ab-4e29-a746-328a6ee1608d", + "description": "Manual", + "user_description": "", + "trigger_type": "manual", + "stats": { + "number_completed": 0 + } + } + ], + "steps": [ + { + "id": "a2ad1433-725d-4ce0-bd2b-857e8bcf3cd1", + "step_type": "call_llm", + "order": 0, + "description": "Call LLM", + "user_description": "", + "stats": { + "number_completed": 0, + "number_failed": 0, + "number_pending": 0 + }, + "parent_step_id": null, + "parent_condition": "", + "action_call_llm": { + "model_name": "kizen/pro", + "business_plugin_app": null, + "prompt": "Summarize the record using the available tools.", + "html_prompt": "

Summarize the record using the available tools.

", + "merge_field_validation": "error_if_required", + "destinations": [ + { + "related_object_field": null, + "field": { + "id": "e7f1ddd1-7861-4328-94b3-1f9008c81f24", + "name": "long_text", + "display_name": "long text", + "deleted": false, + "field_type": "longtext", + "options": [], + "relation": null, + "rating": null + }, + "variable": null, + "conflict_resolution": "overwrite_except_null", + "options": [], + "confidence_threshold": 0.7, + "is_required": false + } + ], + "is_advanced": false, + "mcp_server": { + "id": "bde29351-2a8b-4479-a0d3-c65d8292782e", + "name": "Example Tools", + "api_name": "example_tools", + "is_enabled": true, + "deleted": false, + "tool_count": 7 + }, + "run_tools_as": { + "id": "afa714ac-33c3-49a9-87b0-be75430a7ec6", + "name": "Automation Runner", + "deleted": false + }, + "max_sequential_turns": 10, + "tool_failure_policy": "never_fail" + }, + "action_on_failure": "notify_continue", + "should_skip_execution": false, + "error_notification_severity_level": "inherit", + "is_branch_group_initiator": false + }, + { + "id": "d9a0c41d-af54-4390-9079-dff3a2c67c7c", + "step_type": "call_llm", + "order": 1, + "description": "Call LLM", + "user_description": "", + "stats": { + "number_completed": 0, + "number_failed": 0, + "number_pending": 0 + }, + "parent_step_id": "a2ad1433-725d-4ce0-bd2b-857e8bcf3cd1", + "parent_condition": "", + "action_call_llm": { + "model_name": "kizen/pro", + "business_plugin_app": null, + "prompt": "Summarize the record.", + "html_prompt": "

Summarize the record.

", + "merge_field_validation": "error_if_required", + "destinations": [ + { + "related_object_field": null, + "field": { + "id": "e7f1ddd1-7861-4328-94b3-1f9008c81f24", + "name": "long_text", + "display_name": "long text", + "deleted": false, + "field_type": "longtext", + "options": [], + "relation": null, + "rating": null + }, + "variable": null, + "conflict_resolution": "overwrite_except_null", + "options": [], + "confidence_threshold": 0.7, + "is_required": false + } + ], + "is_advanced": false, + "mcp_server": null, + "run_tools_as": null, + "max_sequential_turns": 25, + "tool_failure_policy": "never_fail" + }, + "action_on_failure": "notify_continue", + "should_skip_execution": false, + "error_notification_severity_level": "inherit", + "is_branch_group_initiator": false + } + ], + "variables": [], + "revision": 5, + "error_notification_email": "", + "skip_non_working_days": false, + "number_active": 0, + "number_paused": 0, + "number_completed": 0, + "priority_rank": 10, + "trigger_throttle_seconds": 0.5, + "created": "2026-10-27T08:54:34.400081-05:00", + "updated": "2026-10-27T08:55:40.471600-05:00", + "debug_next_n_executions": 0, + "error_notification_severity_level": "error" +} diff --git a/tests/test_automation_payloads.py b/tests/test_automation_payloads.py index 1a5f032..8949633 100644 --- a/tests/test_automation_payloads.py +++ b/tests/test_automation_payloads.py @@ -588,6 +588,295 @@ def test_call_llm_related_object_field_without_hop_raises(patch_live_lookups): _build(spec) +# --------------------------------------------------------------------------- +# call_llm MCP server, run-as account, turn limit, tool failure policy +# (wire shapes confirmed live 2026-10-27) +# --------------------------------------------------------------------------- + +_SERVER_ID = "bde29351-0000-4000-8000-000000000001" +_ACCOUNT_ID = "afa714ac-0000-4000-8000-000000000002" +_TWIN_ID = "afa714ac-0000-4000-8000-000000000003" + + +@pytest.fixture +def step_pickers(monkeypatch): + """Serve the call_llm step's two pickers; count the GETs.""" + from kizen_builder.api import automations as api + from kizen_builder.api import client + + class _Client: + def __init__(self, cfg): + pass + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + calls = {"servers": 0, "accounts": 0} + lists = { + "servers": [ + {"id": _SERVER_ID, "name": "Example Tools", "api_name": "example_tools"} + ], + "accounts": [ + {"id": _ACCOUNT_ID, "name": "Automation Runner"}, + {"id": _TWIN_ID, "name": "Twin"}, + {"id": "afa714ac-0000-4000-8000-000000000004", "name": "Twin"}, + ], + } + + def serve(which): + def fetch(_client): + calls[which] += 1 + return lists[which] + + return fetch + + monkeypatch.setattr(client, "KizenClient", _Client) + monkeypatch.setattr(api, "list_mcp_step_options", serve("servers")) + monkeypatch.setattr(api, "list_run_tools_as_options", serve("accounts")) + return {"calls": calls, "lists": lists} + + +def _llm_spec(*blocks: dict) -> dict: + return { + "api_name": "call_llm_tools_test", + "name": "Call LLM Tools Test", + "type": "record_based", + "target_object": "patients", + "steps": [ + { + "key": f"llm{i}", + "step_type": "call_llm", + "order": i, + "parent_key": f"llm{i - 1}" if i else None, + "action_call_llm": {"model_name": "kizen/pro", "prompt": "Hi", **b}, + } + for i, b in enumerate(blocks) + ], + } + + +_TOOL_KEYS = ( + "mcp_server_id", + "run_tools_as_id", + "max_sequential_turns", + "tool_failure_policy", +) + + +def _tool_keys(payload: dict, i: int = 0) -> tuple: + block = payload["steps"][i]["action_call_llm"] + return tuple(block[k] for k in _TOOL_KEYS) + + +def test_call_llm_omitted_tool_keys_build_to_explicit_defaults(patch_live_lookups): + """Omitting both ids on a PUT keeps the server and clears the account, + so the builder always sends the pair, null included.""" + assert _tool_keys(_build(_llm_spec({}))) == (None, None, 25, "never_fail") + + +def test_call_llm_names_resolve_through_the_step_pickers( + patch_live_lookups, step_pickers +): + payload = _build( + _llm_spec( + {"mcp_server": "example_tools", "run_tools_as": "Automation Runner"}, + { + "mcp_server": "example_tools", + "run_tools_as": "Automation Runner", + "max_sequential_turns": 10, + "tool_failure_policy": "never_fail", + }, + ) + ) + assert _tool_keys(payload, 0) == (_SERVER_ID, _ACCOUNT_ID, 25, "never_fail") + assert _tool_keys(payload, 1) == (_SERVER_ID, _ACCOUNT_ID, 10, "never_fail") + assert step_pickers["calls"] == {"servers": 1, "accounts": 1} + + +def test_call_llm_ids_and_read_objects_need_no_lookup(patch_live_lookups): + """A UUID or a read's expanded object never reaches a resolver.""" + from kizen_builder.translate import _SHIM_CTX + + for block in ( + {"mcp_server_id": _SERVER_ID, "run_tools_as_id": _ACCOUNT_ID}, + {"mcp_server": _SERVER_ID, "run_tools_as": _ACCOUNT_ID}, + { + "mcp_server": {"id": _SERVER_ID, "api_name": "example_tools"}, + "run_tools_as": {"id": _ACCOUNT_ID, "name": "Automation Runner"}, + }, + ): + out = _STEP_BUILDERS["call_llm"]( + {"model_name": "kizen/pro", "html_prompt": "

Hi

", **block}, + AutomationDef.model_validate(_llm_spec()), + _SHIM_CTX, + ) + assert (out["mcp_server_id"], out["run_tools_as_id"]) == ( + _SERVER_ID, + _ACCOUNT_ID, + ) + + +@pytest.mark.parametrize( + "block, message", + [ + ( + {"mcp_server": "nope", "run_tools_as": "Automation Runner"}, + "MCP server 'nope' isn't one a Call LLM step can attach in " + "'{env}'. Available: example_tools", + ), + ( + {"mcp_server": "example_tools", "run_tools_as": "Nobody"}, + "service account 'Nobody' can't run a Call LLM step's tools in " + "'{env}'. Available: Automation Runner, Twin, Twin", + ), + ( + {"mcp_server": "example_tools", "run_tools_as": "Twin"}, + "service account name 'Twin' matches 2 accounts in '{env}': " + f"{_TWIN_ID}, afa714ac-0000-4000-8000-000000000004. Use the id instead.", + ), + ( + {"mcp_server": "example_tools", "mcp_server_id": _TWIN_ID}, + "set mcp_server or mcp_server_id, not both", + ), + ( + {"run_tools_as": "Automation Runner", "run_tools_as_id": _TWIN_ID}, + "set run_tools_as or run_tools_as_id, not both", + ), + ( + {"mcp_server": "example_tools"}, + "step 'llm0': mcp_server is set without run_tools_as. Kizen runs an " + "MCP server's tools as a service account, so set both or neither.", + ), + ( + {"run_tools_as_id": _ACCOUNT_ID}, + "step 'llm0': run_tools_as is set without mcp_server. Set both or neither.", + ), + ], + ids=[ + "unknown-server", + "unknown-account", + "ambiguous-account", + "server-twice", + "account-twice", + "server-alone", + "account-alone", + ], +) +def test_call_llm_tool_reference_errors( + patch_live_lookups, step_pickers, block, message +): + env = LiveContext().env + with pytest.raises(PlanError) as e: + _build(_llm_spec(block)) + assert message.format(env=env) in str(e.value) + + +def test_call_llm_both_spellings_agreeing_is_fine(patch_live_lookups, step_pickers): + payload = _build( + _llm_spec( + { + "mcp_server": "example_tools", + "mcp_server_id": _SERVER_ID, + "run_tools_as": {"id": _ACCOUNT_ID}, + "run_tools_as_id": _ACCOUNT_ID, + } + ) + ) + assert _tool_keys(payload)[:2] == (_SERVER_ID, _ACCOUNT_ID) + + +@pytest.mark.parametrize( + "field, value", + [ + ("max_sequential_turns", 0), + ("max_sequential_turns", 32768), + ("tool_failure_policy", "fail_step"), + ], +) +def test_call_llm_tool_settings_are_bounded_in_the_spec(field, value): + from pydantic import ValidationError + + with pytest.raises(ValidationError, match=field): + AutomationDef.model_validate(_llm_spec({field: value})) + + +@pytest.fixture +def live_mcp_automation(monkeypatch, patch_live_lookups): + """The captured call_llm_mcp automation as the live one `update` reads: + step 0 has a server and account, step 1 has neither.""" + import kizen_builder.tools.planners.automations as ma + + raw = load_fixture("automations/call_llm_mcp.raw.json") + raw["custom_object"]["name"] = "patients" + monkeypatch.setattr( + ma, "list_automations", lambda: [{"api_name": raw["api_name"], "id": raw["id"]}] + ) + monkeypatch.setattr(ma, "get_automation", lambda api_name: {"raw": raw}) + return raw + + +def _update_spec(raw: dict, *blocks: dict) -> dict: + spec = _llm_spec(*blocks) + spec.update(api_name=raw["api_name"], name=raw["name"]) + steps = sorted(raw["steps"], key=lambda s: s["order"]) + for step, live in zip(spec["steps"], steps, strict=True): + step["id"] = live["id"] + return spec + + +def test_update_dry_run_names_steps_whose_mcp_server_goes(live_mcp_automation): + """A spec that omits the keys sends nulls, so the update detaches a server + attached in the UI. The preview `--dry-run` prints says which steps.""" + plan = plan_update_automation(_update_spec(live_mcp_automation, {}, {})) + (op,) = plan.operations + assert op.preview["detaches_mcp_server"] == ["llm0"] + assert op.preview["clears_run_tools_as"] == ["llm0"] + + +def test_update_dry_run_is_quiet_when_the_server_stays( + live_mcp_automation, step_pickers +): + step_pickers["lists"]["servers"][0]["id"] = live_mcp_automation["steps"][0][ + "action_call_llm" + ]["mcp_server"]["id"] + step_pickers["lists"]["accounts"][0]["id"] = live_mcp_automation["steps"][0][ + "action_call_llm" + ]["run_tools_as"]["id"] + tools = {"mcp_server": "example_tools", "run_tools_as": "Automation Runner"} + (op,) = plan_update_automation( + _update_spec(live_mcp_automation, tools, {}) + ).operations + assert "detaches_mcp_server" not in op.preview + assert "clears_run_tools_as" not in op.preview + + +def test_steps_edit_call_llm_names_the_server_by_api_name( + patch_live_lookups, step_pickers +): + from kizen_builder.tools.automations import normalize_step_patch + + patch = { + "action_call_llm": { + "model_name": "kizen/pro", + "html_prompt": "

Hi

", + "mcp_server": "example_tools", + "run_tools_as": "Automation Runner", + "max_sequential_turns": 10, + } + } + out = normalize_step_patch(patch, "llm", "call_llm", _LIVE_PAYLOAD, "patients") + block = out["action_call_llm"] + assert tuple(block[k] for k in _TOOL_KEYS) == ( + _SERVER_ID, + _ACCOUNT_ID, + 10, + "never_fail", + ) + + def test_find_relationship_field_auto_detects_unambiguous_hop(monkeypatch): """LiveContext.find_relationship_field is the auto-detect fallback used when a related_object_field destination omits relationship_field_ref.""" diff --git a/tests/test_translate.py b/tests/test_translate.py index 4b52285..17cc7de 100644 --- a/tests/test_translate.py +++ b/tests/test_translate.py @@ -32,6 +32,7 @@ "automations/activity_logged_schedule_activity.raw.json", "automations/archive_record.raw.json", "automations/branching_test.raw.json", + "automations/call_llm_mcp.raw.json", "automations/condition_code_step.raw.json", "automations/condition_variable_rhs.raw.json", "automations/condition_variable_rhs_pipeline.raw.json", @@ -1278,6 +1279,34 @@ def test_validator_flags_skipped_goal_without_branch(kitchen: dict) -> None: ] +def test_call_llm_tool_settings_translate_to_ids() -> None: + """Reads expand mcp_server/run_tools_as; the wire takes bare UUIDs. A + step with no server sends an explicit null pair: a PUT that omits both + keeps the server and clears the account (confirmed live 2026-10-27). + live_to_payload runs the builder with a context that fails on any lookup.""" + raw = load_fixture("automations/call_llm_mcp.raw.json") + steps = sorted(raw["steps"], key=lambda s: s["order"]) + with_tools, bare = (s["action_call_llm"] for s in steps) + payload = live_to_payload(raw) + keys = ( + "mcp_server_id", + "run_tools_as_id", + "max_sequential_turns", + "tool_failure_policy", + ) + assert [_step_block(payload, "call_llm", i)[k] for i in (0, 1) for k in keys] == [ + with_tools["mcp_server"]["id"], + with_tools["run_tools_as"]["id"], + with_tools["max_sequential_turns"], + "never_fail", + None, + None, + 25, + "never_fail", + ] + assert bare["mcp_server"] is None and with_tools["max_sequential_turns"] == 10 + + def test_validator_accepts_plain_fan_out() -> None: """A step with several plain children and no Branch card between them: the server still accepts it, so the group rules must not reject it."""