hitl/wire: rename 'always' decision-type to 'approve_always'

Renames the SurfSense HITL extension decision-type from "always" to
"approve_always" so it sits in the same verb-first family as "approve",
"reject", and "edit". The Python constant is now SURFSENSE_DECISION_APPROVE_ALWAYS;
the wire value, the permission-domain decision_type, and the FE union members
all match (no wire/internal mismatch).

Both the multi_agent_chat permission middleware and the legacy new_chat one
accept the new wire value; the FE types.ts union is updated accordingly.

The "context.always" payload key is intentionally left untouched - it's the
patterns-to-promote field, semantically distinct from the decision type.
This commit is contained in:
CREDO23 2026-05-15 14:47:32 +02:00
parent 6671c91841
commit c8b756ae8f
16 changed files with 85 additions and 75 deletions

View file

@ -1,11 +1,12 @@
"""Translate the unified langchain HITL envelope into permission-domain semantics.
``PermissionMiddleware`` works with the canonical shape
``{decision_type: "once" | "always" | "reject", feedback?: str, edited_args?: dict}``.
``{decision_type: "once" | "approve_always" | "reject", feedback?: str, edited_args?: dict}``.
The wire envelope arriving from langgraph already lives in the LC HITL shape
(parsed once in :mod:`hitl_wire.decision`); this module performs the small
domain mapping (``approve|edit`` ``once``, ``always`` ``always``,
anything else ``reject``) without re-implementing the envelope walk.
domain mapping (``approve|edit`` ``once``, ``approve_always``
``approve_always``, anything else ``reject``) without re-implementing the
envelope walk.
Failing closed: any unrecognised decision becomes ``reject`` (with a warning)
so the middleware never proceeds on ambiguous input.
@ -20,7 +21,7 @@ from app.agents.multi_agent_chat.subagents.shared.hitl.wire import (
LC_DECISION_APPROVE,
LC_DECISION_EDIT,
LC_DECISION_REJECT,
SURFSENSE_DECISION_ALWAYS,
SURFSENSE_DECISION_APPROVE_ALWAYS,
parse_lc_envelope,
)
@ -28,15 +29,15 @@ logger = logging.getLogger(__name__)
# ``approve`` and ``edit`` both mean "let this call go through this once". The
# legacy SurfSense bare-scalar values (``once`` / ``always`` / ``reject``)
# legacy SurfSense bare-scalar values (``once`` / ``approve_always`` / ``reject``)
# pass through unchanged so historical resume payloads still work.
_LC_TO_PERMISSION: dict[str, str] = {
LC_DECISION_APPROVE: "once",
LC_DECISION_EDIT: "once",
SURFSENSE_DECISION_ALWAYS: "always",
SURFSENSE_DECISION_APPROVE_ALWAYS: "approve_always",
LC_DECISION_REJECT: "reject",
"once": "once",
"always": "always",
"approve_always": "approve_always",
"reject": "reject",
}
@ -49,7 +50,7 @@ def normalize_permission_decision(envelope: Any) -> dict[str, Any]:
bare scalar string, or a pre-canonical dict).
Returns:
``{"decision_type": "once"|"always"|"reject"}`` plus optional
``{"decision_type": "once"|"approve_always"|"reject"}`` plus optional
``feedback`` (``reject`` with a user message) and ``edited_args``
(``edit`` reply with non-empty arg overrides).
"""

View file

@ -10,7 +10,7 @@ from app.agents.multi_agent_chat.subagents.shared.hitl.wire import (
LC_DECISION_APPROVE,
LC_DECISION_EDIT,
LC_DECISION_REJECT,
SURFSENSE_DECISION_ALWAYS,
SURFSENSE_DECISION_APPROVE_ALWAYS,
build_lc_hitl_payload,
)
from app.agents.new_chat.permissions import Rule
@ -21,7 +21,7 @@ _PERMISSION_ASK_DECISIONS: list[str] = [
LC_DECISION_APPROVE,
LC_DECISION_REJECT,
LC_DECISION_EDIT,
SURFSENSE_DECISION_ALWAYS,
SURFSENSE_DECISION_APPROVE_ALWAYS,
]

View file

@ -43,7 +43,7 @@ logger = logging.getLogger(__name__)
@dataclass(frozen=True)
class _AlwaysPromotion:
"""A pending request to save an ``always`` decision to the user's trust list."""
"""A pending request to save an ``approve_always`` decision to the user's trust list."""
connector_id: int
tool_name: str
@ -59,15 +59,15 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
to wildcard patterns. Tools without an entry use the bare
tool name as the only pattern.
runtime_ruleset: Mutable :class:`Ruleset` extended in-place when
the user replies ``"always"``. Reused across calls in the
same agent instance so newly-allowed rules apply downstream.
the user replies ``"approve_always"``. Reused across calls in
the same agent instance so newly-allowed rules apply downstream.
always_emit_interrupt_payload: Set ``False`` to make ``ask``
collapse to ``deny`` (for non-interactive deployments).
tools_by_name: Map from tool name to :class:`BaseTool`, used to
decorate ``ask`` interrupts with the tool's description and
MCP metadata for the FE card.
trusted_tool_saver: Async callback invoked on ``always`` decisions
for MCP tools (those whose ``metadata`` carries an
trusted_tool_saver: Async callback invoked on ``approve_always``
decisions for MCP tools (those whose ``metadata`` carries an
``mcp_connector_id``). Without it the promotion only lives
in-memory for the current agent instance.
"""
@ -104,8 +104,9 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
"""Pure decision pass: returns ``(state_update, pending_promotions)``.
Side effects performed here are in-memory only (rule promotion
into ``runtime_ruleset``). DB writes for ``always`` decisions
are queued as ``_AlwaysPromotion`` and flushed by the async hook.
into ``runtime_ruleset``). DB writes for ``approve_always``
decisions are queued as ``_AlwaysPromotion`` and flushed by the
async hook.
"""
del runtime
messages = state.get("messages") or []
@ -155,7 +156,7 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
)
kind = str(decision.get("decision_type") or "reject").lower()
edited_args = decision.get("edited_args")
if kind in ("once", "always"):
if kind in ("once", "approve_always"):
final_call = (
merge_edited_args(call, edited_args)
if isinstance(edited_args, dict) and edited_args
@ -163,7 +164,7 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
)
if final_call is not call:
any_change = True
if kind == "always":
if kind == "approve_always":
persist_always(self._runtime_ruleset, name, patterns)
promotion = self._build_always_promotion(name)
if promotion is not None:

View file

@ -3,8 +3,8 @@
Static rulesets come from the agent factory (defaults, space-scoped,
thread-scoped, etc.). The runtime ruleset is the in-memory one that
:func:`runtime_promote.persist_always` extends when the user replies
``"always"``. Evaluators always see them merged in this order so newly-
promoted rules apply to subsequent calls.
``"approve_always"``. Evaluators always see them merged in this order so
newly-promoted rules apply to subsequent calls.
"""
from __future__ import annotations

View file

@ -1,4 +1,4 @@
"""Promote an ``"always"`` reply into in-memory allow rules.
"""Promote an ``"approve_always"`` reply into in-memory allow rules.
Subsequent calls within the same agent instance match these new rules and
proceed without prompting. Durable persistence (to ``agent_permission_rules``)

View file

@ -11,7 +11,7 @@ from .payload import (
LC_DECISION_APPROVE,
LC_DECISION_EDIT,
LC_DECISION_REJECT,
SURFSENSE_DECISION_ALWAYS,
SURFSENSE_DECISION_APPROVE_ALWAYS,
build_lc_hitl_payload,
)
@ -19,7 +19,7 @@ __all__ = [
"LC_DECISION_APPROVE",
"LC_DECISION_EDIT",
"LC_DECISION_REJECT",
"SURFSENSE_DECISION_ALWAYS",
"SURFSENSE_DECISION_APPROVE_ALWAYS",
"ParsedLcDecision",
"build_lc_hitl_payload",
"parse_lc_envelope",

View file

@ -28,8 +28,8 @@ class ParsedLcDecision:
Attributes:
decision_type: Lower-cased decision identifier ``"approve"``,
``"reject"``, ``"edit"``, ``"always"``, or any custom value the
FE may emit. Callers map this to their own domain semantics.
``"reject"``, ``"edit"``, ``"approve_always"``, or any custom value
the FE may emit. Callers map this to their own domain semantics.
edited_args: Populated only on ``"edit"`` replies that actually carry
args; ``None`` otherwise so callers can use truthiness directly.
message: Free-form user feedback (typically attached to ``"reject"``).
@ -48,9 +48,9 @@ def parse_lc_envelope(envelope: Any) -> ParsedLcDecision:
- ``{"decisions": [{"type": "approve" | "reject" | "edit", ...}]}`` the
langchain HITL standard envelope.
- A bare scalar string (``"once"``, ``"always"``, ``"reject"``) used by
the legacy SurfSense permission wire. We tolerate it so the parser can
sit behind both call sites without a second adapter.
- A bare scalar string (``"once"``, ``"approve_always"``, ``"reject"``)
used by the legacy SurfSense permission wire. We tolerate it so the
parser can sit behind both call sites without a second adapter.
Edit args are read from the standard ``edited_action.args`` first, then
fall back to a flat ``args`` field for legacy compatibility both shapes

View file

@ -17,10 +17,11 @@ LC_DECISION_APPROVE = "approve"
LC_DECISION_REJECT = "reject"
LC_DECISION_EDIT = "edit"
# ``always`` is a SurfSense extension surfaced by ``PermissionMiddleware`` so a
# single click can promote the matched pattern to a runtime allow rule. The FE
# renders an extra button when it appears in ``allowed_decisions``.
SURFSENSE_DECISION_ALWAYS = "always"
# ``approve_always`` is a SurfSense extension surfaced by ``PermissionMiddleware``
# so a single click can promote the matched pattern to a runtime allow rule and
# (for MCP tools) save it to the user's trusted-tools list. The FE renders an
# extra button when it appears in ``allowed_decisions``.
SURFSENSE_DECISION_APPROVE_ALWAYS = "approve_always"
def build_lc_hitl_payload(
@ -41,8 +42,8 @@ def build_lc_hitl_payload(
an empty dict so the FE always has a stable shape to render.
allowed_decisions: Subset of
``[LC_DECISION_APPROVE, LC_DECISION_REJECT, LC_DECISION_EDIT,
SURFSENSE_DECISION_ALWAYS]``. Other values are passed through but
the FE may not render a control for them.
SURFSENSE_DECISION_APPROVE_ALWAYS]``. Other values are passed through
but the FE may not render a control for them.
interrupt_type: SurfSense card discriminator (``"gmail_email_send"``,
``"permission_ask"``, etc.); the FE keys off this to mount the
right card.
@ -80,6 +81,6 @@ __all__ = [
"LC_DECISION_APPROVE",
"LC_DECISION_EDIT",
"LC_DECISION_REJECT",
"SURFSENSE_DECISION_ALWAYS",
"SURFSENSE_DECISION_APPROVE_ALWAYS",
"build_lc_hitl_payload",
]

View file

@ -23,7 +23,7 @@ Operation:
SurfSense shape and LangChain HITL ``{"decisions": [{"type": ...}]}``
replies are accepted via :func:`_normalize_permission_decision`.
- ``once``: proceed.
- ``always``: also persist allow rules for ``request.always`` patterns.
- ``approve_always``: also persist allow rules for ``request.always`` patterns.
- ``reject`` w/o feedback: raise :class:`RejectedError`.
- ``reject`` w/ feedback: raise :class:`CorrectedError`.
5. On ``allow``: proceed unchanged.
@ -90,6 +90,7 @@ _LC_TYPE_TO_PERMISSION_DECISION: dict[str, str] = {
"approve": "once",
"reject": "reject",
"edit": "once",
"approve_always": "approve_always",
}
@ -130,7 +131,7 @@ def _normalize_permission_decision(decision: Any) -> dict[str, Any]:
mapped = _LC_TYPE_TO_PERMISSION_DECISION.get(raw_type)
if mapped is None:
# Tolerate legacy values arriving without ``decision_type`` wrapping.
if raw_type in {"once", "always", "reject"}:
if raw_type in {"once", "approve_always", "reject"}:
mapped = raw_type
else:
logger.warning(
@ -162,8 +163,8 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
of patterns to evaluate. When a tool isn't listed, the bare
tool name is used as the only pattern.
runtime_ruleset: Mutable :class:`Ruleset` that the middleware
extends in-place when the user replies ``"always"`` to an
ask interrupt. Reused across all calls in the same agent
extends in-place when the user replies ``"approve_always"`` to
an ask interrupt. Reused across all calls in the same agent
instance so newly-allowed rules apply to subsequent calls.
always_emit_interrupt_payload: If True, every ask uses the
SurfSense interrupt wire format (default). Set False to
@ -268,7 +269,7 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
for r in rules
],
# Rules of thumb for the frontend: surface the patterns
# the user can promote to "always" with a single reply.
# the user can promote to "approve_always" with a single reply.
"always": patterns,
},
}
@ -287,12 +288,12 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
return _normalize_permission_decision(decision)
def _persist_always(self, tool_name: str, patterns: list[str]) -> None:
"""Promote ``always`` reply into runtime allow rules.
"""Promote ``approve_always`` reply into runtime allow rules.
Persistence to ``agent_permission_rules`` is done by the
streaming layer (``stream_new_chat``) once it observes the
``always`` reply the middleware just keeps an in-memory
copy so subsequent calls in the same stream see the rule.
``approve_always`` reply the middleware just keeps an
in-memory copy so subsequent calls in the same stream see the rule.
"""
for pattern in patterns:
self._runtime_ruleset.rules.append(
@ -377,7 +378,7 @@ class PermissionMiddleware(AgentMiddleware): # type: ignore[type-arg]
kind = str(decision.get("decision_type") or "reject").lower()
if kind == "once":
kept_calls.append(call)
elif kind == "always":
elif kind == "approve_always":
self._persist_always(name, patterns)
kept_calls.append(call)
elif kind == "reject":

View file

@ -225,7 +225,7 @@ async def test_parallel_self_gated_and_middleware_gated_route_and_resume_cleanly
# Resume order: same order the SSE stream would emit (interrupts list).
decision_self = {"type": "approve"}
decision_mw = {"type": "always"}
decision_mw = {"type": "approve_always"}
flat_decisions = [
# Match `pending` order.
decision_self if pending[0][0] == tcid_self else decision_mw,
@ -268,5 +268,5 @@ async def test_parallel_self_gated_and_middleware_gated_route_and_resume_cleanly
assert self_payloads[0]["decision_type"] == "approve"
assert self_payloads[0]["rejected"] is False
# Middleware-gated always → canonical permission shape with always.
assert mw_payloads[0]["decision"] == {"decision_type": "always"}
# Middleware-gated approve_always → canonical permission shape unchanged.
assert mw_payloads[0]["decision"] == {"decision_type": "approve_always"}

View file

@ -64,8 +64,8 @@ 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
# ``always`` must be in the palette so the FE can render the promote button.
assert "always" in review[0]["allowed_decisions"]
# ``approve_always`` must be in the palette so the FE can render the promote button.
assert "approve_always" in review[0]["allowed_decisions"]
assert value.get("interrupt_type") == "permission_ask"
# SurfSense context rides through verbatim for FE explainability.
assert value["context"]["patterns"] == ["rm/*"]
@ -88,18 +88,18 @@ async def test_resume_with_approve_envelope_returns_once_decision():
@pytest.mark.asyncio
async def test_resume_with_always_envelope_projects_to_always():
"""``always`` reply must project unchanged so the middleware can promote the rule."""
async def test_resume_with_approve_always_envelope_projects_unchanged():
"""``approve_always`` reply must project unchanged so the middleware can promote the rule."""
checkpointer = InMemorySaver()
graph = _build_graph_calling_request_permission_decision(checkpointer)
config = {"configurable": {"thread_id": "perm-always"}}
config = {"configurable": {"thread_id": "perm-approve-always"}}
await graph.ainvoke({"messages": [HumanMessage(content="seed")]}, config)
await graph.ainvoke(
Command(resume={"decisions": [{"type": "always"}]}), config
Command(resume={"decisions": [{"type": "approve_always"}]}), config
)
final = await graph.aget_state(config)
assert final.values.get("final_decision") == {"decision_type": "always"}
assert final.values.get("final_decision") == {"decision_type": "approve_always"}
@pytest.mark.asyncio

View file

@ -1,4 +1,4 @@
"""``always`` decisions for MCP tools are saved via the trusted-tool saver."""
"""``approve_always`` decisions for MCP tools are saved via the trusted-tool saver."""
from __future__ import annotations
@ -90,7 +90,7 @@ def _build_graph(pm, tool_name: str):
@pytest.mark.asyncio
async def test_always_decision_saves_mcp_tool_via_callback():
async def test_approve_always_decision_saves_mcp_tool_via_callback():
saved: list[tuple[int, str]] = []
async def trusted_tool_saver(connector_id: int, tool_name: str) -> None:
@ -106,9 +106,11 @@ async def test_always_decision_saves_mcp_tool_via_callback():
assert pm is not None
graph = _build_graph(pm, tool.name)
config = {"configurable": {"thread_id": "always-mcp"}}
config = {"configurable": {"thread_id": "approve-always-mcp"}}
await graph.ainvoke({"messages": [HumanMessage(content="seed")]}, config)
await graph.ainvoke(Command(resume={"decisions": [{"type": "always"}]}), config)
await graph.ainvoke(
Command(resume={"decisions": [{"type": "approve_always"}]}), config
)
assert saved == [(7, "linear_create_issue")]
@ -138,7 +140,7 @@ async def test_once_decision_does_not_save():
@pytest.mark.asyncio
async def test_always_decision_for_native_tool_skips_save():
async def test_approve_always_decision_for_native_tool_skips_save():
"""Native tools have no ``mcp_connector_id`` so there is nowhere to persist trust."""
saved: list[tuple[int, str]] = []
@ -155,15 +157,17 @@ async def test_always_decision_for_native_tool_skips_save():
assert pm is not None
graph = _build_graph(pm, tool.name)
config = {"configurable": {"thread_id": "always-native"}}
config = {"configurable": {"thread_id": "approve-always-native"}}
await graph.ainvoke({"messages": [HumanMessage(content="seed")]}, config)
await graph.ainvoke(Command(resume={"decisions": [{"type": "always"}]}), config)
await graph.ainvoke(
Command(resume={"decisions": [{"type": "approve_always"}]}), config
)
assert saved == []
@pytest.mark.asyncio
async def test_always_decision_with_no_saver_callback_is_a_noop():
async def test_approve_always_decision_with_no_saver_callback_is_a_noop():
"""Anonymous turns build the middleware without a ``trusted_tool_saver``; must not crash."""
tool = _make_mcp_tool(name="linear_create_issue", connector_id=7)
pm = build_permission_mw(
@ -175,6 +179,8 @@ async def test_always_decision_with_no_saver_callback_is_a_noop():
assert pm is not None
graph = _build_graph(pm, tool.name)
config = {"configurable": {"thread_id": "anon-always"}}
config = {"configurable": {"thread_id": "anon-approve-always"}}
await graph.ainvoke({"messages": [HumanMessage(content="seed")]}, config)
await graph.ainvoke(Command(resume={"decisions": [{"type": "always"}]}), config)
await graph.ainvoke(
Command(resume={"decisions": [{"type": "approve_always"}]}), config
)

View file

@ -22,7 +22,7 @@ from app.agents.multi_agent_chat.subagents.shared.hitl.wire import (
LC_DECISION_APPROVE,
LC_DECISION_EDIT,
LC_DECISION_REJECT,
SURFSENSE_DECISION_ALWAYS,
SURFSENSE_DECISION_APPROVE_ALWAYS,
build_lc_hitl_payload,
parse_lc_envelope,
)
@ -83,7 +83,7 @@ class TestBuildLcHitlPayload:
allowed_decisions=[
LC_DECISION_APPROVE,
LC_DECISION_REJECT,
SURFSENSE_DECISION_ALWAYS,
SURFSENSE_DECISION_APPROVE_ALWAYS,
],
interrupt_type="permission_ask",
context=ctx,
@ -111,7 +111,7 @@ class TestParseLcEnvelope:
assert parsed.message is None
def test_bare_scalar_string_passes_through_lowercased(self):
assert parse_lc_envelope("ALWAYS").decision_type == "always"
assert parse_lc_envelope("APPROVE_ALWAYS").decision_type == "approve_always"
assert parse_lc_envelope("once").decision_type == "once"
def test_non_dict_non_string_collapses_to_reject(self):

View file

@ -106,9 +106,9 @@ class TestAsk:
# No new rule persisted
assert mw._runtime_ruleset.rules == []
def test_always_persists_runtime_rule(self) -> None:
def test_approve_always_persists_runtime_rule(self) -> None:
mw = PermissionMiddleware(rulesets=[])
mw._raise_interrupt = lambda **kw: {"decision_type": "always"} # type: ignore[assignment]
mw._raise_interrupt = lambda **kw: {"decision_type": "approve_always"} # type: ignore[assignment]
state = {"messages": [_msg({"name": "send_email", "args": {}, "id": "1"})]}
out = mw.after_model(state, _FakeRuntime())
assert out is None # call kept