From 6416aa63afbc938ce1492c3e938dc2d3c699057c Mon Sep 17 00:00:00 2001 From: now-ing Date: Sun, 6 Sep 2026 21:09:43 +0800 Subject: [PATCH] fix(reward-memory): fail open on outbound scope mismatch instead of crashing sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The outbound guidance hook compared the configured peer_ref against the raw caller agent id while the registry check normalizes it, so an agent id with trailing whitespace or different casing passed the registry gate, failed the raw comparison, and raised ValueError at hook construction — which lark-inbox send/reply surface directly as a traceback, blocking the entire message. A surface whose configured peer_ref drifted to another agent crashed the same way. Normalize the agent id with the same contract the registry uses before comparing, and degrade both build-stage scope mismatches (wrong peer and divergent corpora identity) to returning no hook: absence is the documented fail-open path that preserves the existing sender exactly, so a misconfigured advisory recall can never block delivery. Signed-off-by: now-ing --- loopx/capabilities/reward_memory/outbound.py | 10 +-- tests/capabilities/test_outbound_guidance.py | 67 +++++++++++++++----- 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/loopx/capabilities/reward_memory/outbound.py b/loopx/capabilities/reward_memory/outbound.py index 7f7df2f326..1f3a12942c 100644 --- a/loopx/capabilities/reward_memory/outbound.py +++ b/loopx/capabilities/reward_memory/outbound.py @@ -12,6 +12,7 @@ from pathlib import Path from typing import Any +from ...control_plane.todos.contract import normalize_todo_claimed_by from .experiment import ( resolve_reward_memory_experiment, resolve_reward_memory_surface_config, @@ -34,6 +35,7 @@ def outbound_guidance_hook( raise ValueError("invalid outbound message purpose") if not goal_id or not agent_id: return None + normalized_agent_id = normalize_todo_claimed_by(agent_id) or "" _, config = resolve_reward_memory_experiment( registry_path=registry_path, goal_id=goal_id, agent_id=agent_id ) @@ -57,12 +59,10 @@ def outbound_guidance_hook( "session_ref", ) } - if current["peer_ref"] != f"agent:{agent_id}": - raise ValueError( - "outbound recall requires the exact configured agent scope" - ) + if current["peer_ref"] != f"agent:{normalized_agent_id}": + return None if identity is not None and identity != current: - raise ValueError("outbound recall corpora must share an identity scope") + return None identity = current checkpoints[corpus["corpus_id"]] = { **{k: v for k, v in current.items() if v is not None}, diff --git a/tests/capabilities/test_outbound_guidance.py b/tests/capabilities/test_outbound_guidance.py index 539ff32916..aec3add4c3 100644 --- a/tests/capabilities/test_outbound_guidance.py +++ b/tests/capabilities/test_outbound_guidance.py @@ -79,8 +79,7 @@ def test_disabled_urgent_failure_and_wrong_peer(tmp_path, monkeypatch): assert outbound.outbound_guidance_hook(**kwargs, purpose="urgent")("intent")[ "continue_delivery" ] - with pytest.raises(ValueError, match="agent scope"): - outbound.outbound_guidance_hook(**(kwargs | {"agent_id": "other"})) + assert outbound.outbound_guidance_hook(**(kwargs | {"agent_id": "other"})) is None config["automation"]["automatic_recall"] = False assert outbound.outbound_guidance_hook(**kwargs) is None configure(tmp_path, monkeypatch, unavailable=True) @@ -183,7 +182,9 @@ def run(config, situation, **kw): @pytest.mark.parametrize("command", ["send", "reply"]) -def test_cli_legacy_namespace_preserves_default_off_sender(tmp_path, monkeypatch, command): +def test_cli_legacy_namespace_preserves_default_off_sender( + tmp_path, monkeypatch, command +): from loopx.cli_commands import lark_inbox as cli config, _, project = _fixture(tmp_path, lifecycle=False) @@ -195,27 +196,48 @@ def test_cli_legacy_namespace_preserves_default_off_sender(tmp_path, monkeypatch def send(**kwargs): calls.append(kwargs) - return {"ok": True, "status": "preview_ready", "external_write_performed": False} + return { + "ok": True, + "status": "preview_ready", + "external_write_performed": False, + } monkeypatch.setattr( - cli, "reply_lark_event_inbox" if command == "reply" else "send_lark_inbox_message", send + cli, + "reply_lark_event_inbox" if command == "reply" else "send_lark_inbox_message", + send, ) # Direct callers predating outbound recall do not have the new parser fields. args = argparse.Namespace( - command="lark-inbox", lark_inbox_command=command, - goal_id=None, agent_id=None, message_id="om_reaction_fixture", - route_key="example", text="done", execute=False, provider_preflight=True, + command="lark-inbox", + lark_inbox_command=command, + goal_id=None, + agent_id=None, + message_id="om_reaction_fixture", + route_key="example", + text="done", + execute=False, + provider_preflight=True, ) results = [] - assert cli.handle_lark_inbox_command( - args, registry_path=tmp_path / "registry.json", runtime_root_arg=None, - output_format=lambda *a: "json", - print_payload=lambda payload, *a: results.append(payload), - ) == 0 + assert ( + cli.handle_lark_inbox_command( + args, + registry_path=tmp_path / "registry.json", + runtime_root_arg=None, + output_format=lambda *a: "json", + print_payload=lambda payload, *a: results.append(payload), + ) + == 0 + ) assert len(calls) == 1 assert calls[0] == { - "project": project, "config_path": config, "text": "done", - "execute": False, "provider_preflight": True, "before_send": None, + "project": project, + "config_path": config, + "text": "done", + "execute": False, + "provider_preflight": True, + "before_send": None, **({"message_id": "om_reaction_fixture"} if command == "reply" else {}), } assert results[0]["status"] == "preview_ready" @@ -263,3 +285,18 @@ def test_cli_installs_hook_at_real_sender(tmp_path, monkeypatch, command): assert results[0]["status"] == "agent_review_required" assert results[0]["external_write_performed"] is False assert "not a request for user approval" in cli._render(results[0]) + + +def test_unnormalized_agent_id_and_scope_drift_fail_open(tmp_path, monkeypatch): + """Build-stage scope problems must never crash the send path.""" + + configure(tmp_path, monkeypatch) + kwargs = dict(registry_path=tmp_path / "registry.json", goal_id="goal") + for raw_agent_id in ("pilot", "pilot ", "Pilot"): + hook = outbound.outbound_guidance_hook( + **kwargs, agent_id=raw_agent_id, purpose="progress" + ) + assert callable(hook), ( + f"agent_id={raw_agent_id!r} must normalize to the configured scope" + ) + assert hook("sha256:intent")["status"] == "applied"