fix(agent): close exact approval edge cases

This commit is contained in:
RaresKeY
2026-08-15 07:44:32 +00:00
parent 73a4b10642
commit 105a7c0d96
7 changed files with 254 additions and 36 deletions
+39 -10
View File
@@ -31,7 +31,11 @@ from src.context_compactor import (
)
from src.settings import get_setting
from src.prompt_security import untrusted_context_message
from src.tool_security import blocked_tools_for_owner, plan_mode_disabled_tools
from src.tool_security import (
blocked_tools_for_owner,
email_tool_policy_names,
plan_mode_disabled_tools,
)
from src.tool_policy import GUIDE_ONLY_DIRECTIVE, WEB_TOOL_NAMES, ToolPolicy
from src.tool_capabilities import (
ResultIntegrity,
@@ -5630,7 +5634,40 @@ async def stream_agent_loop(
_ody_notes_finetune_mode
and block.tool_type in {"manage_notes", "manage_calendar", "manage_tasks"}
)
if not security_decision.allowed:
policy_names = email_tool_policy_names(block.tool_type)
blocked_by_tool_policy = bool(
tool_policy
and any(tool_policy.blocks(name) for name in policy_names)
)
blocked_by_disabled_tools = bool(
disabled_tools and not policy_names.isdisjoint(disabled_tools)
)
if (
(blocked_by_tool_policy or blocked_by_disabled_tools)
and not _ody_clamped_tool_allowed
):
if blocked_by_tool_policy:
blocked_name = next(
name for name in policy_names if tool_policy.blocks(name)
)
reason = tool_policy.reason_for(blocked_name)
else:
reason = (
f"Tool '{block.tool_type}' is disabled by the current "
"request policy."
)
desc = f"{block.tool_type}: BLOCKED"
result = {
"error": reason,
"exit_code": 1,
"blocked": True,
"policy": "current_tool_policy",
}
logger.info(
"Tool blocked before approval by current policy: %s",
block.tool_type,
)
elif not security_decision.allowed:
approval_document = (
active_document
if block.tool_type
@@ -5706,14 +5743,6 @@ async def stream_agent_loop(
"Exact approval required before tool start: %s",
block.tool_type,
)
elif tool_policy and tool_policy.blocks(block.tool_type) and not _ody_clamped_tool_allowed:
desc = f"{block.tool_type}: BLOCKED"
result = {
"error": tool_policy.reason_for(block.tool_type),
"exit_code": 1,
"blocked": True,
}
logger.info("Tool blocked before start by policy: %s", block.tool_type)
else:
yield (
f'data: {json.dumps({"type": "tool_start", "tool": block.tool_type, "command": cmd_display, "full_command": full_command, "round": round_num})}\n\n'
+36 -5
View File
@@ -622,6 +622,7 @@ async def run_teacher_inline(
from src.agent_loop import stream_agent_loop
captured_tool_events: List[Dict[str, Any]] = []
captured_text_parts: List[str] = []
captured_metrics: Dict[str, Any] = {}
async for evt_str in stream_agent_loop(
endpoint_url=teacher_url,
@@ -649,6 +650,11 @@ async def run_teacher_inline(
if isinstance(payload, dict):
payload["teacher"] = True
typ = payload.get("type")
if typ == "metrics" and isinstance(payload.get("data"), dict):
# The outer chat route persists only the last metrics
# payload. Keep a copy so any approval produced after the
# recursive teacher run's metrics remains reloadable.
captured_metrics = dict(payload["data"])
if typ == "tool_output":
captured_tool_event = {
"tool": payload.get("tool"),
@@ -750,6 +756,27 @@ async def run_teacher_inline(
"the complete skill definition before it is saved."
),
)
persisted_metrics = dict(captured_metrics)
persisted_tool_events = list(persisted_metrics.get("tool_events") or [])
persisted_round_texts = list(persisted_metrics.get("round_texts") or [])
prior_rounds = [
event.get("round")
for event in persisted_tool_events
if isinstance(event, dict) and isinstance(event.get("round"), int)
]
approval_round = max([len(persisted_round_texts), *prior_rounds, 0]) + 1
approval_tool_event = {
"round": approval_round,
"model": teacher_model,
"tool": "manage_skills",
"command": str(skill.get("name") or "teacher-generated skill"),
"output": "Waiting for an exact user approval.",
"exit_code": None,
"ask_user": approval,
}
persisted_tool_events.append(approval_tool_event)
persisted_metrics["tool_events"] = persisted_tool_events
persisted_metrics.setdefault("model", teacher_model)
yield (
"data: "
+ json.dumps({"delta": "Review the teacher-generated skill before saving it."})
@@ -759,11 +786,7 @@ async def run_teacher_inline(
"data: "
+ json.dumps({
"type": "tool_output",
"tool": "manage_skills",
"command": str(skill.get("name") or "teacher-generated skill"),
"output": "Waiting for an exact user approval.",
"exit_code": None,
"ask_user": approval,
**approval_tool_event,
"teacher": True,
})
+ "\n\n"
@@ -773,3 +796,11 @@ async def run_teacher_inline(
+ json.dumps({"type": "ask_user", "data": approval, "teacher": True})
+ "\n\n"
)
# This must be the final metrics event: chat_routes saves only last_metrics
# when the outer stream reaches [DONE]. Without it, the live approval card
# disappears after a reload even though the server grant remains pending.
yield (
"data: "
+ json.dumps({"type": "metrics", "data": persisted_metrics, "teacher": True})
+ "\n\n"
)
+18 -10
View File
@@ -528,17 +528,25 @@ def tool_result_should_arm_gate(
return False
if tool_result_is_successful(result):
return True
model_visible_keys = (
"error",
"stderr",
"stdout",
"output",
"content",
"response",
"results",
"images",
# ``format_tool_result`` serializes every additional structured field, so
# a fixed allowlist here would inevitably miss model-visible payloads such
# as ``details``, ``events``, or provider-specific response keys. Exclude
# only status/policy controls that carry no producer content; any other
# non-empty field crosses the same integrity boundary even on failure.
non_content_keys = frozenset(
{
"approval_required",
"blocked",
"exit_code",
"policy",
"success",
"untrusted_content",
}
)
return any(
key not in non_content_keys and value not in (None, "", [], {}, ())
for key, value in result.items()
)
return any(result.get(key) not in (None, "", [], {}, ()) for key in model_visible_keys)
POST_EXTERNAL_BLOCKED_EFFECTS = frozenset(