diff --git a/CHANGELOG.md b/CHANGELOG.md index 22855e59..2121544b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -434,6 +434,10 @@ and versions follow [Semantic Versioning](https://semver.org/). - A task that repeats the same failing step three times now pauses as stuck and asks what to do, instead of failing. ### Fixed +- **Approval cards no longer show Sentient's notes to itself.** When the browser (or another built-in app) asked + before clicking, the card's small print showed the instructions written for the model, cut off mid-word ("browser: + Click the element with this ref ..."). It now shows a short plain label when there is one, like "Purchase" or + "Unlocks a door", and otherwise the card's own line (#398). - **A page watch always checks the page.** "Track this page and tell me when a new event is added" could be set to start only when something arrived in your calendar or smart home, so it never looked at the page. A check that reads a page, a search or the news now runs on a timer: every hour, or as often as you asked (#389). diff --git a/docs/API.md b/docs/API.md index 14cfa4dd..ea598abf 100644 --- a/docs/API.md +++ b/docs/API.md @@ -37,7 +37,7 @@ All carry `session_id` and `turn_id`. | `tool_call` | `call_id`, `name`, `arguments` | | `tool_progress` | `call_id`, `name`, `kind: stdout\|stderr\|status\|frame\|subagent`, `text?`, `image?`, `data?` (section 10) | | `tool_result` | `call_id`, `name`, `result`, `is_error`, `duration_ms` (a call the user declined has `result: {error, declined: true}`; a call a "Hand it to me" rule handed to the user has `result: HandOff`, section 2, #140), `check?: OutcomeCheck` (what code found the call really did, section 4 "Checking the outcome", #165; absent for calls that aren't checked) | -| `approval_request` | `approval_id`, `call_id`, `name`, `arguments`, `risk: read\|write\|send\|exec` (effective risk), `reason`, `risk_label?`, `target?`, `untrusted?` (section 10), `warning?` (what a second look found off, section 10, #143), `choices?: string[]` (the answers offered, in order, section 2 "Scoped answers"), `grant_minutes?` (how long `allow_timed` lasts), `preview?: ActionPreview` (what will be sent or changed, the way a person reads it, section 10, #142) | +| `approval_request` | `approval_id`, `call_id`, `name`, `arguments`, `risk: read\|write\|send\|exec` (effective risk), `reason` (a short plain label for the card: `risk_label` when it says more than the risk itself, such as `"Purchase"` or `"Unlocks a door"`, else `""`; never the tool's description, which is written for the model, #398), `risk_label?`, `target?`, `untrusted?` (section 10), `warning?` (what a second look found off, section 10, #143), `choices?: string[]` (the answers offered, in order, section 2 "Scoped answers"), `grant_minutes?` (how long `allow_timed` lasts), `preview?: ActionPreview` (what will be sent or changed, the way a person reads it, section 10, #142) | | `undo_window` | `undo_id`, `call_id`, `name`, `seconds`, `until` (ISO time it ends), `preview?: ActionPreview`: an email, message or post waits `seconds` before it runs, so it can still be undone (section 10, #142). Its `tool_result` follows when the window ends; after Undo that result is `{error, undone: true}` and nothing was sent | | `user_interjection` | `text` (a steer message the model just received, section 10) | | `steer_ack` | `session_id`, `queued`, `client_id` (echo; no `turn_id`) | diff --git a/sentient/agent/loop.py b/sentient/agent/loop.py index 786e376f..15dacd9c 100644 --- a/sentient/agent/loop.py +++ b/sentient/agent/loop.py @@ -102,6 +102,7 @@ Risk, Tool, ToolContext, + approval_reason, bind_call, describe_call, effective_risk, @@ -1263,7 +1264,7 @@ async def _ask(self, p: _CallPlan, ctx: ToolContext, ev: dict) -> AsyncIterator[ self.approvals.create(approval_id, offered) yield ApprovalRequest( approval_id=approval_id, call_id=p.tc.id, name=p.tc.name, arguments=p.tc.arguments, - risk=p.risk.name, reason=f"{p.tool.plugin}: {p.tool.description[:160]}", + risk=p.risk.name, reason=approval_reason(wording, p.risk), risk_label=wording["risk_label"], target=wording["target"], untrusted=p.untrusted or None, warning=p.warning or None, choices=offered, grant_minutes=self.approvals.config.grant_minutes, diff --git a/sentient/tools/base.py b/sentient/tools/base.py index cab5424a..27a8d561 100644 --- a/sentient/tools/base.py +++ b/sentient/tools/base.py @@ -281,6 +281,14 @@ async def describe_call(tool: Tool, arguments: dict, ctx: ToolContext, risk: Ris return out +def approval_reason(wording: dict, risk: Risk) -> str: + """The reason line of an approval card (#398): the label a ``describe_fn`` gave when it says more than the risk + alone ("Purchase", "Deletes", "Unlocks a door", "Takes a photo"), else empty, so the card writes its own line. + Never the tool's description: that is written for the model ("Click the element with this ref ...").""" + label = str((wording or {}).get("risk_label") or "").strip() + return "" if label == RISK_LABELS.get(Risk(risk), "") else label + + def _params_model_from_signature(fn: Callable, name: str) -> type[BaseModel]: sig = inspect.signature(fn) hints = typing.get_type_hints(fn, include_extras=True) diff --git a/tests/test_approval_reason.py b/tests/test_approval_reason.py new file mode 100644 index 00000000..e8c0021a --- /dev/null +++ b/tests/test_approval_reason.py @@ -0,0 +1,56 @@ +"""The reason line of an approval card is never the tool's instructions for the model (#398).""" + +from __future__ import annotations + +from sentient.app import SentientApp +from sentient.llm.events import ApprovalRequest +from sentient.tools.base import Risk, ToolContext, ToolPlugin, approval_reason, tool +from tests.conftest import FakeProvider, tool_call + +MODEL_TEXT = "Click the element with this ref (from snapshot), e.g. \"e12\". Never type passwords. Buttons that buy" + + +def test_only_a_label_that_says_more_than_the_risk_is_a_reason(): + assert approval_reason({"risk_label": "Sends", "target": "Send message"}, Risk.send) == "" + assert approval_reason({"risk_label": "Changes something"}, Risk.write) == "" + assert approval_reason({"risk_label": "Purchase", "target": "Place order"}, Risk.send) == "Purchase" + assert approval_reason({"risk_label": "Unlocks a door"}, Risk.send) == "Unlocks a door" + assert approval_reason({}, Risk.send) == "" + + +async def test_an_approval_request_carries_no_model_instructions(config, isolated_home): + def describe(arguments, ctx): + return {"risk_label": "Purchase" if "order" in arguments.get("label", "").lower() else "Sends", + "target": arguments.get("label")} + + # the description is what the model reads, as a plugin tool's docstring would be + @tool("page_press", risk=Risk.send, description=MODEL_TEXT, describe_fn=describe) + async def page_press(ctx: ToolContext, label: str) -> dict: + return {"pressed": label} + + class Page(ToolPlugin): + id = "page" + display_name = "Page" + tools = [page_press] + + config.tools.approvals.mode = "ask" + llm = FakeProvider(replies=[ + [tool_call("page_press", label="Send message")], "not sent", + [tool_call("page_press", label="Place order")], "not bought", + ]) + s = await SentientApp(config, llm=llm, db_path=isolated_home / "reason.db").start() + try: + s.registry.register(Page()) + sid = await s.store.create_session(channel="desktop") + asked: list[ApprovalRequest] = [] + for text in ("send it", "order it"): + async for ev in s.agent.run_turn(sid, text, channel="desktop"): + if isinstance(ev, ApprovalRequest): + asked.append(ev) + s.approvals.resolve(ev.approval_id, "deny") + assert [(a.reason, a.risk_label, a.target) for a in asked] == [ + ("", "Sends", "Send message"), ("Purchase", "Purchase", "Place order"), + ] + assert all("Click the element" not in a.model_dump_json() for a in asked) + finally: + await s.stop()