From a4e81a8f2f3d9cb8ea456c4876566fee599de2dd Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 2 Jul 2026 00:41:54 -0700 Subject: [PATCH] [eric] hitl: Approve All persists always-allow so mid-run identical commands stop re-prompting --- backend/tests/test_accept_all_midrun.py | 46 +++++++++++++++++++ .../app/components/overlays/DynamicIsland.tsx | 5 +- .../app/pages/Dashboard/cards/AgentCard.tsx | 3 +- 3 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 backend/tests/test_accept_all_midrun.py diff --git a/backend/tests/test_accept_all_midrun.py b/backend/tests/test_accept_all_midrun.py new file mode 100644 index 00000000..f9329495 --- /dev/null +++ b/backend/tests/test_accept_all_midrun.py @@ -0,0 +1,46 @@ +"""'Approve All' mid-run invariant: an allow carrying set_always_allow must make the +NEXT identical tool call auto-approve inside the SAME run. The gate reads the live +builtin_perms dict (effective_policy) and the approval path writes through the same +dict (set_tool_policy), so the policy written at approval time is visible to the very +next call without waiting for the next turn's from-disk reload. Pins the loop the +frontend 'Approve All' buttons now rely on (they send set_always_allow=true).""" + +import pytest + +from backend.apps.agents.manager.permissions import decision + + +@pytest.fixture() +def p_isolated_persistence(monkeypatch): + """Redirect the disk-persistence half of set_tool_policy into memory.""" + persisted: dict = {"perms": {}} + monkeypatch.setattr(decision, "load_all_tools", lambda: []) + monkeypatch.setattr(decision, "load_builtin_permissions", lambda: dict(persisted["perms"])) + monkeypatch.setattr(decision, "save_builtin_permissions", lambda perms: persisted.update(perms=dict(perms))) + return persisted + + +def test_always_allow_applies_to_next_call_same_run(p_isolated_persistence): + live_perms = {"Bash": "ask"} + assert decision.effective_policy("Bash", live_perms, {}) == "ask" + decision.set_tool_policy("Bash", "always_allow", live_perms) + # The very next identical call in the SAME run reads the live dict and auto-approves. + assert decision.effective_policy("Bash", live_perms, {}) == "always_allow" + # And it persisted, so the next turn's from-disk reload keeps it. + assert p_isolated_persistence["perms"]["Bash"] == "always_allow" + + +def test_always_allow_namespaced_builtin_uses_inner_slot(p_isolated_persistence): + # Our browser/invoke delegation tools live in builtin_permissions under the INNER name; a write through the namespaced name must land where the next read looks. + live_perms: dict = {} + name = "mcp__openswarm-browser-agent__BrowserAgent" + decision.set_tool_policy(name, "always_allow", live_perms) + assert live_perms == {"BrowserAgent": "always_allow"} + assert decision.effective_policy(name, live_perms, {}) == "always_allow" + + +def test_plain_allow_leaves_policy_untouched(p_isolated_persistence): + # A one-time allow (no set_always_allow) never calls set_tool_policy; the policy stays 'ask' and the next call prompts again. Guards against silently widening plain approves. + live_perms = {"Bash": "ask"} + assert decision.effective_policy("Bash", live_perms, {}) == "ask" + assert p_isolated_persistence["perms"] == {} diff --git a/frontend/src/app/components/overlays/DynamicIsland.tsx b/frontend/src/app/components/overlays/DynamicIsland.tsx index 2ccfce7e..bc2bdd80 100644 --- a/frontend/src/app/components/overlays/DynamicIsland.tsx +++ b/frontend/src/app/components/overlays/DynamicIsland.tsx @@ -435,7 +435,8 @@ const DynamicIsland: React.FC = () => { for (const g of groups) { for (const req of g.approvals) { if (req.tool_name !== 'AskUserQuestion') { - dispatch(handleApproval({ requestId: req.id, behavior: 'allow' })); + // setAlwaysAllow so the SAME command mid-run stops re-prompting: the backend writes the policy into the live in-run snapshot, a plain allow only clears the pending request. + dispatch(handleApproval({ requestId: req.id, behavior: 'allow', setAlwaysAllow: true })); } } } @@ -792,7 +793,7 @@ const CompactActionablePill: React.FC<{ {remainingCount > 1 && !isIntervention && ( - + { e.stopPropagation(); onApproveAll(); }} diff --git a/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx b/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx index 99a19c6d..7f585b43 100644 --- a/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx +++ b/frontend/src/app/pages/Dashboard/cards/AgentCard.tsx @@ -1165,7 +1165,8 @@ const AgentCard: React.FC = ({ startIcon={} onClick={() => { for (const req of session.pending_approvals) { - if (req.tool_name !== 'AskUserQuestion') dispatch(handleApproval({ requestId: req.id, behavior: 'allow' })); + // setAlwaysAllow so the SAME command mid-run stops re-prompting: the backend writes the policy into the live in-run snapshot, a plain allow only clears the pending request. + if (req.tool_name !== 'AskUserQuestion') dispatch(handleApproval({ requestId: req.id, behavior: 'allow', setAlwaysAllow: true })); } }} sx={{