From 1040a7efcc7424b52aab1e00f4659f209a848050 Mon Sep 17 00:00:00 2001 From: Pavel Date: Wed, 19 Aug 2026 23:08:46 +0200 Subject: [PATCH] tool content loss fixes --- application/api/user/agents/portability.py | 99 ++++++++++ frontend/src/modals/ImportAgentModal.tsx | 30 +-- frontend/src/modals/ImportSpecModal.tsx | 31 ++-- tests/api/test_agent_portability.py | 206 +++++++++++++++++++++ 4 files changed, 337 insertions(+), 29 deletions(-) diff --git a/application/api/user/agents/portability.py b/application/api/user/agents/portability.py index e29ab510..905a36d5 100644 --- a/application/api/user/agents/portability.py +++ b/application/api/user/agents/portability.py @@ -174,6 +174,101 @@ def _safe_export_config(stored_config: dict, config_requirements: dict) -> dict: return safe +# Param-type sections an action may carry. Fixed ``value``s are exported only +# for ``parameters`` (declared function args, treated as non-secret throughout: +# returned to the client, shown in the actions editor). The free-form HTTP +# types routinely hold credentials (an ``Authorization`` header fixed as a +# value), so for those only the ``filled_by_llm`` flag travels — same posture +# as ``_safe_export_config`` dropping free-form config. +_ACTION_PARAM_TYPES = ("parameters", "query_params", "headers", "body") +_ACTION_VALUE_SAFE_TYPES = frozenset({"parameters"}) + + +def _export_action_overrides(actions) -> list: + """Compact per-action customizations that deviate from factory defaults. + + ``transform_actions`` initializes every action ``active`` with every param + ``filled_by_llm: true, value: ""`` — anything else is a user customization + (a fixed ``chat_id``, a disabled action) that must survive export, or the + imported tool silently reverts to "let the LLM fill everything". + """ + out = [] + for action in actions or []: + if not isinstance(action, dict) or not action.get("name"): + continue + override: dict[str, Any] = {"name": action["name"]} + customized = False + if action.get("active", True) is False: + override["active"] = False + customized = True + for param_type in _ACTION_PARAM_TYPES: + section = action.get(param_type) + props = section.get("properties") if isinstance(section, dict) else None + if not isinstance(props, dict): + continue + for pname, details in props.items(): + if not isinstance(details, dict): + continue + filled = details.get("filled_by_llm", True) + value = details.get("value", "") + if filled and not value: + continue + entry: dict[str, Any] = {"filled_by_llm": bool(filled)} + if param_type in _ACTION_VALUE_SAFE_TYPES: + entry["value"] = value + override.setdefault(param_type, {})[pname] = entry + customized = True + if customized: + out.append(override) + return out + + +def _apply_action_overrides(base_actions: list, overrides, tool_type: str, warnings: list) -> list: + """Overlay exported customizations onto freshly transformed live metadata. + + Matched strictly by (action name, param-type, param name): the YAML + contributes flags and scalar values only, never schema — a hand-edited + file can't inject a doctored action definition — and the merge survives + tool-version drift. Unknown names are skipped with a warning. Values are + accepted only for the ``parameters`` type, mirroring export. + """ + if not isinstance(overrides, list) or not overrides: + return base_actions + by_name = {a.get("name"): a for a in base_actions if isinstance(a, dict)} + for override in overrides: + if not isinstance(override, dict): + continue + name = override.get("name") or "" + action = by_name.get(name) + if action is None: + warnings.append( + f"Tool '{tool_type}' action '{name}' no longer exists; customization skipped" + ) + continue + if override.get("active") is False: + action["active"] = False + for param_type in _ACTION_PARAM_TYPES: + override_props = override.get(param_type) + if not isinstance(override_props, dict): + continue + section = action.get(param_type) + props = section.get("properties") if isinstance(section, dict) else {} + for pname, entry in override_props.items(): + details = props.get(pname) if isinstance(props, dict) else None + if not isinstance(details, dict) or not isinstance(entry, dict): + warnings.append( + f"Tool '{tool_type}' action '{name}' has no parameter " + f"'{pname}'; customization skipped" + ) + continue + details["filled_by_llm"] = bool(entry.get("filled_by_llm", True)) + if param_type in _ACTION_VALUE_SAFE_TYPES: + value = entry.get("value", "") + if isinstance(value, (str, int, float, bool)) or value is None: + details["value"] = "" if value is None else value + return base_actions + + # --------------------------------------------------------------------------- # Export # --------------------------------------------------------------------------- @@ -236,6 +331,9 @@ def _serialize_tools(conn, agent: dict, user: str) -> list: "config": _safe_export_config(row.get("config") or {}, requirements), "ref": tid_str, } + action_overrides = _export_action_overrides(row.get("actions")) + if action_overrides: + entry["actions"] = action_overrides if requires_secrets: entry["requires_secrets"] = requires_secrets out.append(entry) @@ -839,6 +937,7 @@ def _create_tool_from_spec(conn, user: str, tool: dict, secrets: dict, warnings: warnings.append(f"Tool '{tool_type}' secret encryption failed; not created") return None actions = transform_actions(inst.get_actions_metadata() or []) + actions = _apply_action_overrides(actions, tool.get("actions"), tool_type, warnings) created = UserToolsRepository(conn).create( user, tool_type, diff --git a/frontend/src/modals/ImportAgentModal.tsx b/frontend/src/modals/ImportAgentModal.tsx index 7a46a35a..2c4fa399 100644 --- a/frontend/src/modals/ImportAgentModal.tsx +++ b/frontend/src/modals/ImportAgentModal.tsx @@ -1,5 +1,6 @@ import { AlertTriangle, CheckCircle2 } from 'lucide-react'; -import { useRef, useState } from 'react'; +import { useState } from 'react'; +import { useDropzone } from 'react-dropzone'; import { useTranslation } from 'react-i18next'; import { useSelector } from 'react-redux'; import { useNavigate } from 'react-router-dom'; @@ -74,7 +75,6 @@ export default function ImportAgentModal({ const token = useSelector(selectToken); const sourceDocs = useSelector(selectSourceDocs); const navigate = useNavigate(); - const fileInputRef = useRef(null); const [yamlText, setYamlText] = useState(''); const [fileName, setFileName] = useState(''); @@ -112,9 +112,7 @@ export default function ImportAgentModal({ reset(); }; - const handleFileChange = async (e: React.ChangeEvent) => { - const selectedFile = e.target.files?.[0]; - if (!selectedFile) return; + const processFile = async (selectedFile: File) => { const name = selectedFile.name.toLowerCase(); if (!name.endsWith('.yaml') && !name.endsWith('.yml')) { setError(t('modals.importAgent.invalidFileType')); @@ -126,6 +124,13 @@ export default function ImportAgentModal({ setYamlText(await selectedFile.text()); }; + const { getRootProps, getInputProps, isDragActive } = useDropzone({ + onDrop: (acceptedFiles: File[]) => { + if (acceptedFiles[0]) processFile(acceptedFiles[0]); + }, + multiple: false, + }); + const handleAnalyze = async () => { if (!yamlText) return; setLoading(true); @@ -306,8 +311,11 @@ export default function ImportAgentModal({ {t('modals.importAgent.description')}

fileInputRef.current?.click()} - className="border-border hover:border-primary flex cursor-pointer flex-col items-center justify-center rounded-xl border-2 border-dashed p-8 transition-colors" + {...getRootProps({ + className: `border-border hover:border-primary flex cursor-pointer flex-col items-center justify-center rounded-xl border-2 border-dashed p-8 transition-colors ${ + isDragActive ? 'border-primary' : '' + }`, + })} > {fileName || t('modals.importAgent.dropzoneText')}

+
- {error &&

{error}

} ) : ( diff --git a/frontend/src/modals/ImportSpecModal.tsx b/frontend/src/modals/ImportSpecModal.tsx index 69cf382d..3da09444 100644 --- a/frontend/src/modals/ImportSpecModal.tsx +++ b/frontend/src/modals/ImportSpecModal.tsx @@ -1,4 +1,5 @@ -import { useRef, useState } from 'react'; +import { useState } from 'react'; +import { useDropzone } from 'react-dropzone'; import { useTranslation } from 'react-i18next'; import { useSelector } from 'react-redux'; @@ -36,7 +37,6 @@ export default function ImportSpecModal({ }: ImportSpecModalProps) { const { t } = useTranslation(); const token = useSelector(selectToken); - const fileInputRef = useRef(null); const [file, setFile] = useState(null); const [loading, setLoading] = useState(false); @@ -57,10 +57,7 @@ export default function ImportSpecModal({ setBaseUrl(''); }; - const handleFileChange = (e: React.ChangeEvent) => { - const selectedFile = e.target.files?.[0]; - if (!selectedFile) return; - + const processFile = (selectedFile: File) => { const validExtensions = ['.json', '.yaml', '.yml']; const hasValidExtension = validExtensions.some((ext) => selectedFile.name.toLowerCase().endsWith(ext), @@ -76,6 +73,13 @@ export default function ImportSpecModal({ setParsedResult(null); }; + const { getRootProps, getInputProps, isDragActive } = useDropzone({ + onDrop: (acceptedFiles: File[]) => { + if (acceptedFiles[0]) processFile(acceptedFiles[0]); + }, + multiple: false, + }); + const handleParse = async () => { if (!file) return; @@ -196,8 +200,11 @@ export default function ImportSpecModal({

fileInputRef.current?.click()} - className="border-border dark:border-border hover:border-primary dark:hover:border-primary flex cursor-pointer flex-col items-center justify-center rounded-xl border-2 border-dashed p-8 transition-colors" + {...getRootProps({ + className: `border-border dark:border-border hover:border-primary dark:hover:border-primary flex cursor-pointer flex-col items-center justify-center rounded-xl border-2 border-dashed p-8 transition-colors ${ + isDragActive ? 'border-primary' : '' + }`, + })} > {t('modals.importSpec.supportedFormats')}

- +
{error && ( diff --git a/tests/api/test_agent_portability.py b/tests/api/test_agent_portability.py index eae9ae1e..3732339c 100644 --- a/tests/api/test_agent_portability.py +++ b/tests/api/test_agent_portability.py @@ -497,3 +497,209 @@ def test_tool_skipped_when_encryption_fails(pg_conn, monkeypatch): agent = AgentsRepository(pg_conn).get(result["agent_id"], user) assert agent["tools"] == [] assert any("encryption failed" in w for w in result["warnings"]) + + +# --------------------------------------------------------------------------- +# Action customizations (per-parameter fixed values / filled_by_llm flags) +# --------------------------------------------------------------------------- + + +_SEND_MESSAGE_META = { + "name": "telegram_send_message", + "description": "Send a message", + "parameters": { + "type": "object", + "properties": { + "chat_id": {"type": "string", "description": "Chat id"}, + "text": {"type": "string", "description": "Message text"}, + }, + "required": ["chat_id", "text"], + }, +} +_GET_UPDATES_META = { + "name": "telegram_get_updates", + "description": "Poll updates", + "parameters": {"type": "object", "properties": {}, "required": []}, +} + + +def _fake_telegram(monkeypatch): + import copy as _copy + + fake_tool = Mock() + fake_tool.get_config_requirements.return_value = { + "token": {"secret": True, "required": True, "label": "Bot token"} + } + fake_tool.get_actions_metadata.side_effect = lambda: _copy.deepcopy( + [_SEND_MESSAGE_META, _GET_UPDATES_META] + ) + monkeypatch.setattr( + "application.api.user.agents.portability._tool_instance", + lambda tool_type: fake_tool, + ) + return fake_tool + + +def _stored_telegram_actions(): + """The shape ``update_tool_actions`` persists after the user fixes chat_id + and disables the updates action.""" + return [ + { + "name": "telegram_send_message", + "description": "Send a message", + "active": True, + "parameters": { + "type": "object", + "properties": { + "chat_id": { + "type": "string", + "description": "Chat id", + "filled_by_llm": False, + "value": "123456789", + }, + "text": { + "type": "string", + "description": "Message text", + "filled_by_llm": True, + "value": "", + }, + }, + "required": ["chat_id", "text"], + }, + }, + { + "name": "telegram_get_updates", + "description": "Poll updates", + "active": False, + "parameters": {"type": "object", "properties": {}, "required": []}, + }, + ] + + +def test_action_customizations_round_trip(pg_conn, monkeypatch): + user = "u_actions" + _fake_telegram(monkeypatch) + tool = UserToolsRepository(pg_conn).create( + user, + "telegram", + config={"encrypted_credentials": "BLOB"}, + custom_name="My TG", + display_name="Telegram", + description="tg", + config_requirements={"token": {"secret": True, "required": True}}, + actions=_stored_telegram_actions(), + ) + agent = AgentsRepository(pg_conn).create( + user, "TG Bot", "published", description="d", tools=[str(tool["id"])] + ) + + export = serialize_agent(pg_conn, agent, user) + overrides = export["spec"]["tools"][0]["actions"] + # Only deviations from factory defaults travel: the fixed chat_id and the + # disabled action — the default-state ``text`` param does not. + assert overrides == [ + { + "name": "telegram_send_message", + "parameters": {"chat_id": {"filled_by_llm": False, "value": "123456789"}}, + }, + {"name": "telegram_get_updates", "active": False}, + ] + + # Import as a fresh user (tool re-created with the supplied secret). + doc = parse_agent_yaml(agent_to_yaml(export)) + importer = "u_actions_2" + result = apply_import( + pg_conn, + importer, + doc, + resolution={"tools": {"tool-0": {"decision": "create", "secrets": {"token": "tok"}}}}, + ) + imported_agent = AgentsRepository(pg_conn).get(result["agent_id"], importer) + row = UserToolsRepository(pg_conn).get_any(imported_agent["tools"][0], importer) + by_name = {a["name"]: a for a in row["actions"]} + chat_id = by_name["telegram_send_message"]["parameters"]["properties"]["chat_id"] + text = by_name["telegram_send_message"]["parameters"]["properties"]["text"] + assert chat_id["filled_by_llm"] is False + assert chat_id["value"] == "123456789" + assert text["filled_by_llm"] is True and text["value"] == "" + assert by_name["telegram_get_updates"]["active"] is False + assert by_name["telegram_send_message"]["active"] is True + + +def test_action_header_values_never_exported(pg_conn): + user = "u_actions_hdr" + tool = UserToolsRepository(pg_conn).create( + user, + "custom_http", + config={}, + custom_name="HTTP", + display_name="HTTP", + description="", + config_requirements={}, + actions=[ + { + "name": "call", + "active": True, + "headers": { + "properties": { + "Authorization": { + "type": "string", + "filled_by_llm": False, + "value": "Bearer sk-live-XYZ", + } + } + }, + } + ], + ) + agent = AgentsRepository(pg_conn).create( + user, "Hdr Bot", "published", description="d", tools=[str(tool["id"])] + ) + + export = serialize_agent(pg_conn, agent, user) + text = agent_to_yaml(export) + assert "Bearer sk-live-XYZ" not in text + # The flag still travels so the imported action keeps its shape. + override = export["spec"]["tools"][0]["actions"][0] + assert override["headers"]["Authorization"] == {"filled_by_llm": False} + + +def test_action_override_for_removed_action_warns(pg_conn, monkeypatch): + user = "u_actions_gone" + _fake_telegram(monkeypatch) + doc = _doc( + name="Drifted Bot", + _slug="drifted", + tools=[ + { + "type": "telegram", + "name": "My TG", + "config": {}, + "requires_secrets": ["token"], + "actions": [ + {"name": "telegram_removed_action", "active": False}, + { + "name": "telegram_send_message", + "parameters": { + "gone_param": {"filled_by_llm": False, "value": "x"}, + "chat_id": {"filled_by_llm": False, "value": "42"}, + }, + }, + ], + } + ], + ) + result = apply_import( + pg_conn, + user, + doc, + resolution={"tools": {"tool-0": {"secrets": {"token": "tok"}}}}, + ) + agent = AgentsRepository(pg_conn).get(result["agent_id"], user) + row = UserToolsRepository(pg_conn).get_any(agent["tools"][0], user) + by_name = {a["name"]: a for a in row["actions"]} + # The surviving customization applied; the drifted ones warned and skipped. + chat_id = by_name["telegram_send_message"]["parameters"]["properties"]["chat_id"] + assert chat_id["filled_by_llm"] is False and chat_id["value"] == "42" + assert any("telegram_removed_action" in w for w in result["warnings"]) + assert any("gone_param" in w for w in result["warnings"])