Skip to content

Model a Call LLM step's MCP server and run-as account - #99

Merged
2 commits merged into
mainfrom
bug/call-llm-steps-refused
Oct 27, 2026
Merged

2 commits merged into
mainfrom
bug/call-llm-steps-refused

Conversation

@jbedient-kizen

@jbedient-kizen jbedient-kizen commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Since Kizen's 2026-10-26 release every Call LLM step reads back max_sequential_turns and tool_failure_policy, even with no MCP server attached, plus mcp_server and run_tools_as when one is. The CLI refuses step settings it doesn't model, so show, diff, steps, activate, deactivate, move and roundtrip stopped on every automation with a Call LLM step. There's also a wrong write: when automations update runs a spec that carries a step's id and omits these keys, Kizen keeps the server, clears the run-as account, and stores a step its own validator then rejects.

Solution

_step_call_llm models all four keys. On the wire they're mcp_server_id and run_tools_as_id (bare UUIDs), max_sequential_turns and tool_failure_policy. Translate passes the read's expanded objects through as ids with no lookups, and the builder always sends all four. A spec names the server by api_name and the account by name, or either by UUID, in create, update, diff, steps add and steps edit. Names resolve against the step's own pickers (/api/mcp-servers/step-options and /api/automation2/automations/run-tools-as-options), through a generic lazy LiveContext._step_picker cache. --dry-run refuses a server without an account, and the reverse, as Kizen does.

Testing

Local checks after merging main: lint, format, typecheck, pytest (2856 passed, 4 skipped, 1 xfailed) and the CLI-tree diff, passed: lint format typecheck test extra_checks.

Live on a test tenant: a scratch automation created from a spec naming the server by api_name and the account by name read back with both set on one step and null on the other, 25/never_fail on both. Half specs and an unknown server were refused at --dry-run with the exact texts. diff against a spec taken from show → no changes; steps edit naming the server by api_name, roundtrip --execute (zero drift), activate and deactivate all wrote cleanly. update --dry-run from a spec without the keys named the step losing its server and account. Read only: two existing automations with Call LLM steps, which main refuses, now render. Drift -k "mcp or schema_keys_are_honoured": the MCP read-back, put-back and read-key classification pass; the one failure is the pre-existing schedule_activity schema key, which also fails on main. The UI hasn't been checked; the CLI-written step reads back with the same expanded objects as a UI-built one.

Design notes / tradeoffs

A spec that omits the keys sends null/null/25/never_fail, so update detaches a server attached in the UI. That's how PUT treats every other step setting, and omitting them isn't a safe "keep" (it stores the broken step above). The update preview names each step that loses its server or account, and diff shows it.

The pickers are wrapped here rather than reusing #100 (the mcp-servers command group)'s or #101 (the service-accounts command group)'s list functions: those lists include accounts the step can't use (the plugin service account is a 400), and this fix shouldn't wait on two new command groups.

Overlaps: #95 (the webhook-auth fix) and #96 (the loops fix) also change tools/planners/automations.py, docs/specs/automation.md and CHANGELOG.md, and #96 (the loops fix) adds to tests/drift/test_roundtrip_automations.py. The second of these to merge should need a small conflict fix. #95 (the webhook-auth fix)'s integration-secret lookup could later move onto the step-picker cache added here. The merge from main also refreshes scripts/cli-tree-baseline.txt for #92's new kizen upgrade --check help text, which main's baseline lacks; every PR in this set carries that same line.

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.
@jbedient-kizen

Copy link
Copy Markdown
Contributor Author

Included in the 0.4.0 release PR #102; merge that instead.

@sangyeT sangyeT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the four new call_llm keys, the picker-backed name resolution (_tool_ref_uuid, cached per LiveContext), the pairing check, the always-send-null fix for the server/account mismatch, and the detaches_mcp_server preview. One thing to be aware of rather than a blocker: an update from a spec that omits mcp_server/run_tools_as detaches a server attached in the UI, which is the opposite choice from #95 keeping live webhook auth. It's documented and surfaced in --dry-run, so approving; worth a sentence in the release notes if teams re-apply older specs.

@jbedient-kizen jbedient-kizen closed this pull request by merging all changes into main in 39beea1 Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants