permissions/ask: gate 'approve_always' palette entry on MCP-ness

Only MCP tools have a persistence target for 'approve_always' (the
connector's trusted-tools list); for native tools the decision lives
only in the in-memory runtime ruleset. Reflect that in the wire palette
so the FE can stay a pure renderer of allowed_decisions instead of
peeking at context.mcp_connector_id to decide whether to show the
'Always Allow' button.

The backend still accepts an 'approve_always' reply for any tool kind
(in-memory promotion is harmless), it just doesn't advertise it when
there's nowhere to persist.
This commit is contained in:
CREDO23 2026-05-15 14:54:16 +02:00
parent c8b756ae8f
commit 98b6977c68
3 changed files with 71 additions and 8 deletions

View file

@ -17,14 +17,21 @@ from app.agents.new_chat.permissions import Rule
PERMISSION_ASK_INTERRUPT_TYPE = "permission_ask"
_PERMISSION_ASK_DECISIONS: list[str] = [
_BASE_PERMISSION_ASK_DECISIONS: list[str] = [
LC_DECISION_APPROVE,
LC_DECISION_REJECT,
LC_DECISION_EDIT,
SURFSENSE_DECISION_APPROVE_ALWAYS,
]
def _is_mcp_tool(tool: BaseTool | None) -> bool:
"""An MCP tool advertises a connector id in its langchain metadata."""
if tool is None:
return False
metadata = getattr(tool, "metadata", None) or {}
return metadata.get("mcp_connector_id") is not None
def _card_fields_from_tool(tool: BaseTool | None) -> dict[str, Any]:
"""Project the FE card's tool-scoped fields out of a BaseTool."""
if tool is None:
@ -52,10 +59,15 @@ def build_permission_ask_payload(
) -> dict[str, Any]:
"""Build the permission-ask interrupt payload.
``tool`` carries the FE card's tool-scoped fields (description, MCP
connector). When omitted the card still renders, just without the
"Always Allow against this connected account" surface.
``approve_always`` is added to the palette only for MCP tools, since that
is the only case where the user's choice can persist beyond the current
agent instance (saved to the connector's trusted-tools list). Native
tools fall back to the once/reject/edit triad.
"""
allowed_decisions = list(_BASE_PERMISSION_ASK_DECISIONS)
if _is_mcp_tool(tool):
allowed_decisions.append(SURFSENSE_DECISION_APPROVE_ALWAYS)
context: dict[str, Any] = {
"patterns": patterns,
"rules": [
@ -68,7 +80,7 @@ def build_permission_ask_payload(
return build_lc_hitl_payload(
tool_name=tool_name,
args=args,
allowed_decisions=_PERMISSION_ASK_DECISIONS,
allowed_decisions=allowed_decisions,
interrupt_type=PERMISSION_ASK_INTERRUPT_TYPE,
context=context,
)

View file

@ -64,8 +64,12 @@ async def test_permission_ask_payload_uses_lc_hitl_shape():
], f"REGRESSION: permission ask reverted to legacy shape; got {value!r}"
review = value.get("review_configs")
assert isinstance(review, list) and len(review) == 1
# ``approve_always`` must be in the palette so the FE can render the promote button.
assert "approve_always" in review[0]["allowed_decisions"]
palette = review[0]["allowed_decisions"]
# Native tool (no ``tool=`` argument): the palette must include the
# once/reject/edit triad. ``approve_always`` is gated on MCP-ness and
# therefore *omitted* here — palette content per tool kind is
# exercised in ``test_permission_ask_mcp_context``.
assert "approve" in palette and "reject" in palette and "edit" in palette
assert value.get("interrupt_type") == "permission_ask"
# SurfSense context rides through verbatim for FE explainability.
assert value["context"]["patterns"] == ["rm/*"]

View file

@ -81,6 +81,53 @@ def test_payload_omits_tool_fields_when_tool_is_none():
assert "tool_description" not in ctx
def test_palette_includes_approve_always_for_mcp_tool():
"""Saving to the connector's trusted-tools list is only possible for MCP tools."""
tool = _make_mcp_tool(
name="linear_create_issue", connector_id=42, connector_name="Linear"
)
palette = build_permission_ask_payload(
tool_name=tool.name,
args={},
patterns=[tool.name],
rules=[_ask_rule(tool.name)],
tool=tool,
)["review_configs"][0]["allowed_decisions"]
assert "approve_always" in palette
def test_palette_excludes_approve_always_for_native_tool():
"""Native tools have no place to persist trust, so don't offer the button."""
native = StructuredTool(
name="rm",
description="Remove a file.",
coroutine=_noop,
args_schema=_NoArgs,
metadata={"hitl": True},
)
palette = build_permission_ask_payload(
tool_name=native.name,
args={"path": "/tmp/x"},
patterns=[native.name],
rules=[_ask_rule(native.name)],
tool=native,
)["review_configs"][0]["allowed_decisions"]
assert "approve_always" not in palette
assert palette == ["approve", "reject", "edit"]
def test_palette_excludes_approve_always_when_tool_is_none():
"""Without a tool object the middleware can't tell — fall back to the safe triad."""
palette = build_permission_ask_payload(
tool_name="rm",
args={"path": "/tmp/x"},
patterns=["rm"],
rules=[_ask_rule("rm")],
tool=None,
)["review_configs"][0]["allowed_decisions"]
assert palette == ["approve", "reject", "edit"]
def test_payload_omits_falsy_mcp_metadata_fields():
tool = StructuredTool(
name="anon_tool",