diff --git a/docsgpt/agents/tool_executor.py b/docsgpt/agents/tool_executor.py index 51df4a3d..046467a9 100644 --- a/docsgpt/agents/tool_executor.py +++ b/docsgpt/agents/tool_executor.py @@ -14,7 +14,7 @@ from docsgpt.agents.default_tools import ( synthesized_default_tools, ) from docsgpt import tracing -from docsgpt.agents.tool_pins import iter_parameters, llm_fills, resolve_arguments, sent_arguments, shown_arguments +from docsgpt.agents.tool_pins import iter_parameters, llm_fills, resolve_arguments, sent_arguments from docsgpt.agents.tools.tool_action_parser import ToolActionParser from docsgpt.agents.tools.tool_manager import ToolManager from docsgpt.guardrails.types import Stage as GuardrailStage, resolve_tool_result @@ -1513,7 +1513,7 @@ class ToolExecutor: body, parameters = sections["body"], sections["parameters"] # The chat shows what was sent; ``arguments`` keeps what the model # asked for, which is what a later turn replays to it. - sent = shown_arguments(sections) + sent = sent_arguments(action_data, call_args, self._connection_parameters(tool_data)) if sent != (call_args if isinstance(call_args, dict) else {}): tool_call_data["sent_arguments"] = sent @@ -1843,7 +1843,9 @@ class ToolExecutor: # Keys the client needs that are not part of the fixed shape below. They are # small and optional, and are copied only when present so an ordinary tool # call does not grow null columns in every persisted row. - _PRESERVED_TOOL_CALL_KEYS = ("artifacts", "device_id", "connector_key", "connector_name", "access") + _PRESERVED_TOOL_CALL_KEYS = ( + "artifacts", "device_id", "connector_key", "connector_name", "access", "sent_arguments", + ) def get_truncated_tool_calls(self) -> List[Dict]: """Project tool calls into the shape that is streamed and persisted. diff --git a/docsgpt/agents/tool_pins.py b/docsgpt/agents/tool_pins.py index 51efb800..f546ccc5 100644 --- a/docsgpt/agents/tool_pins.py +++ b/docsgpt/agents/tool_pins.py @@ -104,17 +104,8 @@ def resolve_arguments( return resolved -def shown_arguments(sections: Mapping[str, Mapping[str, Any]]) -> dict: - """One flat ``{parameter: value}`` view of resolved sections, for the chat. - - Headers are left out: they may carry a pinned secret (an API key the - owner fixed), and the chat is shown to whoever runs the agent. - """ - shown: dict = {} - for section in PARAM_SECTIONS: - if section != "headers": - shown.update(sections.get(section) or {}) - return shown +#: What the chat shows in place of a value the owner fixed. +FIXED_MASK = "(fixed)" def sent_arguments( @@ -122,12 +113,29 @@ def sent_arguments( llm_arguments: Mapping[str, Any], connection_pins: Optional[Mapping[str, Any]] = None, ) -> dict: - """What a call sends, flattened for display (see :func:`shown_arguments`). + """What a call sends, flattened for the chat (the approval card, a finished call). - The approval card and the finished call show this rather than what the - model asked for, which fixed values may have replaced. + It follows :func:`resolve_arguments`, but a value the owner fixed shows + as :data:`FIXED_MASK`: it may be a secret (an API key in a query), and + the chat is shown to whoever runs the agent, over the API or a widget + too. A value the connection sets (Telegram's default chat) is the + account's own setting and shows as it is. Headers are left out. """ - return shown_arguments(resolve_arguments(action, llm_arguments, connection_pins)) + connection_pins = connection_pins or {} + shown: dict = {} + for section, name, details in iter_parameters(action): + if section == "headers": + continue + if name in connection_pins: + shown[name] = connection_pins[name] + elif not llm_fills(details): + if has_value(details): + shown[name] = FIXED_MASK + elif name in llm_arguments: + shown[name] = llm_arguments[name] + elif has_value(details): + shown[name] = details["value"] + return shown def coerce_value(details: Mapping, value: Any) -> Any: diff --git a/tests/agents/test_tool_pins.py b/tests/agents/test_tool_pins.py index a3366099..39ba1d2a 100644 --- a/tests/agents/test_tool_pins.py +++ b/tests/agents/test_tool_pins.py @@ -254,11 +254,15 @@ class TestArgumentsShownForACall: call.id = "c1" return executor.check_pause(_tools(action), call, "MockLLM") - def test_approval_card_shows_the_fixed_value(self, monkeypatch): + def test_approval_card_marks_a_fixed_value_without_revealing_it(self, monkeypatch): + """A value the owner fixed may be a secret, and the chat is shown to + whoever runs the agent: the card says it is fixed, not what it is.""" + from docsgpt.agents.tool_pins import FIXED_MASK + action = {**_action(text=_llm(), chat_id=_pinned("111")), "require_approval": True} pending = self._pause(ToolExecutor(user="u"), action, {"text": "hi", "chat_id": "666"}, monkeypatch) assert pending["pause_type"] == "awaiting_approval" - assert pending["sent_arguments"] == {"text": "hi", "chat_id": "111"} + assert pending["sent_arguments"] == {"text": "hi", "chat_id": FIXED_MASK} # What the model asked stays as it was: resuming replays it to the model. assert pending["arguments"] == {"text": "hi", "chat_id": "666"} @@ -267,19 +271,36 @@ class TestArgumentsShownForACall: pending = self._pause(ToolExecutor(user="u"), action, {"text": "hi"}, monkeypatch) assert "sent_arguments" not in pending - def test_a_finished_call_records_what_was_sent(self, mock_tool_manager, monkeypatch): + def test_a_finished_call_records_what_was_sent_and_keeps_it(self, mock_tool_manager, monkeypatch): + from docsgpt.agents.tool_pins import FIXED_MASK + executor = ToolExecutor(user="u") action = _action(text=_llm(), chat_id=_pinned("111")) _run(executor, _tools(action), {"text": "hi", "chat_id": "666"}, monkeypatch) recorded = executor.tool_calls[-1] - assert recorded["sent_arguments"] == {"text": "hi", "chat_id": "111"} + assert recorded["sent_arguments"] == {"text": "hi", "chat_id": FIXED_MASK} assert recorded["arguments"] == {"text": "hi", "chat_id": "666"} + # Saved with the conversation, so a reload shows the same. + (saved,) = executor.get_truncated_tool_calls() + assert saved["sent_arguments"] == {"text": "hi", "chat_id": FIXED_MASK} - def test_headers_are_never_shown(self): + def test_a_value_the_connection_sets_is_shown(self, monkeypatch): + """Telegram's default chat is the account's own setting, not a secret.""" from docsgpt.agents.tool_pins import sent_arguments + action = _action(text=_llm(), chat_id=_llm()) + assert sent_arguments(action, {"text": "hi", "chat_id": "666"}, {"chat_id": "111"}) == { + "text": "hi", "chat_id": "111", + } + + def test_fixed_query_and_body_values_and_headers_stay_hidden(self): + from docsgpt.agents.tool_pins import FIXED_MASK, sent_arguments + action = { "headers": {"properties": {"Authorization": _pinned("Bearer secret")}}, - "query_params": {"properties": {"id": _llm()}}, + "query_params": {"properties": {"id": _llm(), "api_key": _pinned("sk-query")}}, + "body": {"properties": {"token": _pinned("sk-body")}}, } - assert sent_arguments(action, {"id": "1"}) == {"id": "1"} + shown = sent_arguments(action, {"id": "1"}) + assert shown == {"id": "1", "api_key": FIXED_MASK, "token": FIXED_MASK} + assert "secret" not in str(shown) and "sk-" not in str(shown)