mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-13 16:14:10 +00:00
tool content loss fixes
This commit is contained in:
1 parent
3c78540931
commit
1040a7efcc
4 files changed
+337
-29
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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<HTMLInputElement>(null);
|
||||
|
||||
const [yamlText, setYamlText] = useState<string>('');
|
||||
const [fileName, setFileName] = useState<string>('');
|
||||
@@ -112,9 +112,7 @@ export default function ImportAgentModal({
|
||||
reset();
|
||||
};
|
||||
|
||||
const handleFileChange = async (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
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')}
|
||||
</p>
|
||||
<div
|
||||
onClick={() => 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' : ''
|
||||
}`,
|
||||
})}
|
||||
>
|
||||
<img
|
||||
src={Upload}
|
||||
@@ -317,14 +325,8 @@ export default function ImportAgentModal({
|
||||
<p className="text-foreground text-sm font-medium">
|
||||
{fileName || t('modals.importAgent.dropzoneText')}
|
||||
</p>
|
||||
<input {...getInputProps({ accept: '.yaml,.yml' })} />
|
||||
</div>
|
||||
<input
|
||||
ref={fileInputRef}
|
||||
type="file"
|
||||
accept=".yaml,.yml"
|
||||
onChange={handleFileChange}
|
||||
className="hidden"
|
||||
/>
|
||||
{error && <p className="text-destructive text-sm">{error}</p>}
|
||||
</div>
|
||||
) : (
|
||||
|
||||
@@ -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<HTMLInputElement>(null);
|
||||
|
||||
const [file, setFile] = useState<File | null>(null);
|
||||
const [loading, setLoading] = useState(false);
|
||||
@@ -57,10 +57,7 @@ export default function ImportSpecModal({
|
||||
setBaseUrl('');
|
||||
};
|
||||
|
||||
const handleFileChange = (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
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({
|
||||
</p>
|
||||
|
||||
<div
|
||||
onClick={() => 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' : ''
|
||||
}`,
|
||||
})}
|
||||
>
|
||||
<img
|
||||
src={Upload}
|
||||
@@ -210,13 +217,7 @@ export default function ImportSpecModal({
|
||||
<p className="mt-1 text-xs text-gray-500 dark:text-gray-400">
|
||||
{t('modals.importSpec.supportedFormats')}
|
||||
</p>
|
||||
<input
|
||||
ref={fileInputRef}
|
||||
type="file"
|
||||
accept=".json,.yaml,.yml"
|
||||
onChange={handleFileChange}
|
||||
className="hidden"
|
||||
/>
|
||||
<input {...getInputProps({ accept: '.json,.yaml,.yml' })} />
|
||||
</div>
|
||||
|
||||
{error && (
|
||||
|
||||
@@ -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"])
|
||||
Reference in new issue
Block a user