diff --git a/docsgpt/agents/tool_executor.py b/docsgpt/agents/tool_executor.py index 4db23d1e..95f8ba6b 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 +from docsgpt.agents.tool_pins import iter_parameters, llm_fills, resolve_arguments, sent_arguments, shown_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 @@ -1102,6 +1102,12 @@ class ToolExecutor: "pause_type": "awaiting_approval", "thought_signature": getattr(call, "thought_signature", None), } + # The card shows what will be sent: fixed values replace what the + # model asked for. ``arguments`` stays as the model sent it, since + # resuming replays it to the model, which never sees fixed values. + sent = sent_arguments(action_data, arguments, self._connection_parameters(tool_data)) + if action_data and sent != arguments: + payload["sent_arguments"] = sent # Surface the device id so the approval UI can offer a # "don't ask again" sticky-pattern action for remote devices. if tool_data.get("name") == "remote_device": @@ -1482,6 +1488,11 @@ class ToolExecutor: sections = resolve_arguments(action_data, call_args, self._connection_parameters(tool_data)) query_params, headers = sections["query_params"], sections["headers"] 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) + if sent != (call_args if isinstance(call_args, dict) else {}): + tool_call_data["sent_arguments"] = sent # Load tool (with caching) from docsgpt.connectors.service import ConnectionUnavailable diff --git a/docsgpt/agents/tool_pins.py b/docsgpt/agents/tool_pins.py index f473f175..51efb800 100644 --- a/docsgpt/agents/tool_pins.py +++ b/docsgpt/agents/tool_pins.py @@ -104,6 +104,32 @@ 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 + + +def sent_arguments( + action: Mapping, + llm_arguments: Mapping[str, Any], + connection_pins: Optional[Mapping[str, Any]] = None, +) -> dict: + """What a call sends, flattened for display (see :func:`shown_arguments`). + + The approval card and the finished call show this rather than what the + model asked for, which fixed values may have replaced. + """ + return shown_arguments(resolve_arguments(action, llm_arguments, connection_pins)) + + def coerce_value(details: Mapping, value: Any) -> Any: """Check a value for a pin against the parameter's declared type. diff --git a/docsgpt/llm/handlers/base.py b/docsgpt/llm/handlers/base.py index a8090056..1d432f81 100644 --- a/docsgpt/llm/handlers/base.py +++ b/docsgpt/llm/handlers/base.py @@ -1240,6 +1240,9 @@ class LLMHandler(ABC): # can wire the sticky "don't ask again" button. if pause_info.get("device_id"): pause_data["device_id"] = pause_info["device_id"] + # What will be sent once fixed values replace the model's. + if pause_info.get("sent_arguments") is not None: + pause_data["sent_arguments"] = pause_info["sent_arguments"] # A connection-backed tool whose account needs signing in: the # approval card becomes a Connect card. if pause_info.get("connection_required"): diff --git a/frontend/src/conversation/AnswerFlow.tsx b/frontend/src/conversation/AnswerFlow.tsx index 6d04f089..7e208297 100644 --- a/frontend/src/conversation/AnswerFlow.tsx +++ b/frontend/src/conversation/AnswerFlow.tsx @@ -17,7 +17,7 @@ import { AnswerSegment, getAnswerSegments } from './answerSegments'; import MarkdownAnswer from './MarkdownAnswer'; import { type SandboxArtifact } from './sandboxLinks'; import StreamingStatusLine from './StreamingStatusLine'; -import { ToolCallsType } from './types'; +import { shownArguments, ToolCallsType } from './types'; import { isWikiWriteCall } from './wikiToolCall'; import { cn } from '@/lib/utils'; @@ -310,10 +310,10 @@ function InlineToolCallChip({

- {JSON.stringify(toolCall.arguments ?? {}, null, 2)} + {JSON.stringify(shownArguments(toolCall), null, 2)}

{ (b) => b.textContent?.trim() === text, ) as HTMLButtonElement; + it('shows the fixed values a call will send, not what the model asked', async () => { + await render( + {}} + />, + ); + expect(container.textContent).toContain('"chat_id":"111"'); + expect(container.textContent).not.toContain('666'); + const details = container.querySelector( + `button[aria-label="${tr('conversation.toolApproval.details')}"]`, + ) as HTMLButtonElement; + await act(async () => details.click()); + expect(container.querySelector('pre')?.textContent).toContain('"111"'); + expect(container.textContent).not.toContain('666'); + }); + it('renders the approval bar on Button variants and disables Approve while a deny reason is typed', async () => { await render( 60 ? argPreview.slice(0, 57) + '...' : argPreview; @@ -899,7 +899,7 @@ function ToolCallApprovalBar({ 
-                {JSON.stringify(toolCall.arguments, null, 2)}
+                {JSON.stringify(shownArguments(toolCall), null, 2)}
diff --git a/frontend/src/conversation/types/index.ts b/frontend/src/conversation/types/index.ts index 4e925c84..e119bf9c 100644 --- a/frontend/src/conversation/types/index.ts +++ b/frontend/src/conversation/types/index.ts @@ -3,6 +3,9 @@ export type ToolCallsType = { action_name: string; call_id: string; arguments: Record; + // What the call sends once fixed values replace what the model asked for; + // absent when they are the same. The chat shows this. + sent_arguments?: Record; result?: Record; error?: string; status?: @@ -39,3 +42,7 @@ export type ToolCallsType = { connector_name?: string | null; access?: 'read' | 'write' | null; }; + +/** The arguments to show for a call: what it sends, not what the model asked. */ +export const shownArguments = (toolCall: ToolCallsType): Record => + toolCall.sent_arguments ?? toolCall.arguments ?? {}; diff --git a/tests/agents/test_tool_pins.py b/tests/agents/test_tool_pins.py index 383262da..a3366099 100644 --- a/tests/agents/test_tool_pins.py +++ b/tests/agents/test_tool_pins.py @@ -238,3 +238,48 @@ class TestPinHelpers: carried = carry_pins(old, fresh) assert carried["parameters"]["properties"]["team"]["value"] == "ENG" assert carried["parameters"]["properties"]["team"]["filled_by_llm"] is False + + +@pytest.mark.unit +class TestArgumentsShownForACall: + """What the chat shows for a call is what is sent, not what the model asked.""" + + def _pause(self, executor, action, call_args, monkeypatch): + monkeypatch.setattr( + "docsgpt.agents.tool_executor.ToolActionParser", + lambda _cls, **kw: Mock(parse_args=Mock(return_value=("t1", "telegram_send_message", call_args))), + ) + call = Mock() + call.name = "telegram_send_message" + call.id = "c1" + return executor.check_pause(_tools(action), call, "MockLLM") + + def test_approval_card_shows_the_fixed_value(self, monkeypatch): + 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"} + # What the model asked stays as it was: resuming replays it to the model. + assert pending["arguments"] == {"text": "hi", "chat_id": "666"} + + def test_no_separate_arguments_when_nothing_changes(self, monkeypatch): + action = {**_action(text=_llm()), "require_approval": True} + 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): + 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["arguments"] == {"text": "hi", "chat_id": "666"} + + def test_headers_are_never_shown(self): + from docsgpt.agents.tool_pins import sent_arguments + + action = { + "headers": {"properties": {"Authorization": _pinned("Bearer secret")}}, + "query_params": {"properties": {"id": _llm()}}, + } + assert sent_arguments(action, {"id": "1"}) == {"id": "1"} diff --git a/tests/test_tool_approval.py b/tests/test_tool_approval.py index ee1f99b0..de9e2d91 100644 --- a/tests/test_tool_approval.py +++ b/tests/test_tool_approval.py @@ -215,6 +215,30 @@ class TestHandlerApprovalPause: ] assert len(approval_events) == 1 + def test_approval_event_carries_what_will_be_sent(self): + handler = ConcreteHandler() + agent = self._make_agent({ + "call_id": "c1", + "name": "send_msg_0", + "tool_name": "telegram", + "tool_id": "0", + "action_name": "send_msg", + "arguments": {"text": "hello", "chat_id": "666"}, + "sent_arguments": {"text": "hello", "chat_id": "111"}, + "pause_type": "awaiting_approval", + "thought_signature": None, + }) + call = ToolCall(id="c1", name="send_msg_0", arguments='{"text": "hello"}') + gen = handler.handle_tool_calls(agent, [call], {"0": {"name": "telegram"}}, []) + events = [] + try: + while True: + events.append(next(gen)) + except StopIteration: + pass + (event,) = [e for e in events if e.get("data", {}).get("status") == "awaiting_approval"] + assert event["data"]["sent_arguments"] == {"text": "hello", "chat_id": "111"} + def test_mixed_normal_and_approval(self): """First tool runs normally, second needs approval.""" handler = ConcreteHandler()