feat: tool approval declarations (v9)
Agent definitions gain tool_approvals: known tool -> ask/auto/deny. Unknown tool/policy is a 400, and required+deny is a 400 contradiction. The declaration is persisted (tasks.agent_tool_ approvals, PG + migrate), echoed in GET and the creation response (via to_summary), and resolved for registry definitions -- fixing a latent drop where resolve_registry_agent re-parsed without new fields. At dispatch the worker audits the declaration and fails fast on a defensive required+deny re-check (v3 tools-gate shape, runner never called); goal/coordinator tasks reject the field at creation like instructions. effective_tool_approvals maps a declaration + host global approval to the actual host answer, with ask_honoured always False: host answers elicitation with one global switch and carries no tool identity, and the approvals queue is record-only with no runtime ask channel, so per-tool interception and credential scope are explicitly NOT claimed.
This commit is contained in:
1 parent
cd33a929ca
commit
a51919f24b
6 files changed
+441
-6
No files matched your search
+101
-3
@@ -87,6 +87,45 @@ field, but deliberately as REQUIRED tools, not an allowlist:
|
||||
seeded earlier by another worker persists in the shared tenant
|
||||
home). Local/cloud workers skip the check and are documented as
|
||||
such, not as "tools verified".
|
||||
|
||||
v9 (tool approvals, this file plus server/worker changes) adds a
|
||||
``tool_approvals`` field, and it is deliberately narrower than
|
||||
"per-tool approval enforcement" -- the execution layer cannot
|
||||
honour that today, and this track says so instead of claiming it:
|
||||
|
||||
* Declaration: ``tool_approvals`` maps a known tool to one of
|
||||
``ask`` / ``auto`` / ``deny``. Unknown tools and unknown
|
||||
policies are 400s. A tool declared both required (``tools``)
|
||||
and ``deny`` is a 400: a task that must use a tool it must
|
||||
never be allowed to use is a contradiction, not a policy.
|
||||
* What is NOT claimed: per-tool interception in host.py.
|
||||
``host.py _answer_server_request`` answers every elicitation
|
||||
with one global switch (``approval == "auto"`` accepts,
|
||||
anything else declines) and the elicitation message carries
|
||||
no tool/server identity this codebase can rely on, so host
|
||||
cannot tell which tool is asking. The approvals queue
|
||||
(``cloud/approvals.py``) is record-only today: host runs in
|
||||
a worker-spawned subprocess with no control-plane channel,
|
||||
so there is no "ask the user" path at runtime. Credential
|
||||
scope is NOT part of this field either: MCP credentials live
|
||||
in the tenant codex-home config / env, per-project, not
|
||||
per-task, and declaring a scope here without a channel to
|
||||
enforce it would be the same kind of lie.
|
||||
* What IS executed: the declaration is validated at creation,
|
||||
persisted (tasks.agent_tool_approvals, JSON object, NULL =
|
||||
undeclared), echoed in GET and the creation response, and
|
||||
at dispatch the worker audits the declaration together with
|
||||
its effective outcome under today's host behaviour
|
||||
(``effective_tool_approvals``): ``auto`` + host-auto accepts,
|
||||
everything else declines, and ``ask`` is reported as
|
||||
"decline (no ask channel)" -- never as "will ask". The
|
||||
worker also re-checks the required+deny contradiction at
|
||||
dispatch (defence in depth for rows written before v9 or
|
||||
outside the API) and fails fast before any model call, in
|
||||
the v3 tools-gate shape. Goal and coordinator tasks reject
|
||||
tool_approvals at creation, like instructions: their turns
|
||||
do not pass the declaration through, so accepting it there
|
||||
would be a silent drop.
|
||||
"""
|
||||
|
||||
import json
|
||||
@@ -102,8 +141,9 @@ EFFORTS = ("low", "medium", "high", "xhigh", "max")
|
||||
APPROVALS = ("ask", "auto", "full")
|
||||
EXECUTORS = ("local", "docker", "cloud")
|
||||
_TOP_LEVEL_KEYS = ("name", "version", "description", "instructions",
|
||||
"skills", "tools", "defaults")
|
||||
"skills", "tools", "tool_approvals", "defaults")
|
||||
KNOWN_TOOLS = ("websearch", "imagegen", "playwright")
|
||||
TOOL_APPROVAL_POLICIES = ("ask", "auto", "deny")
|
||||
_DEFAULTS_KEYS = ("model", "effort", "approval", "executor", "plan")
|
||||
SKILLS_DIR = Path(__file__).resolve().parent / "skills"
|
||||
AGENTS_DIR = Path(__file__).resolve().parent.parent / "agents"
|
||||
@@ -139,13 +179,14 @@ class AgentDefinition:
|
||||
|
||||
def __init__(self, name, version=None, description=None,
|
||||
instructions=None, skills=None, tools=None,
|
||||
defaults=None):
|
||||
tool_approvals=None, defaults=None):
|
||||
self.name = name
|
||||
self.version = version
|
||||
self.description = description
|
||||
self.instructions = instructions
|
||||
self.skills = list(skills or [])
|
||||
self.tools = list(tools or [])
|
||||
self.tool_approvals = dict(tool_approvals or {})
|
||||
self.defaults = dict(defaults or {})
|
||||
|
||||
def to_summary(self):
|
||||
@@ -155,9 +196,39 @@ class AgentDefinition:
|
||||
"version": self.version,
|
||||
"skills": list(self.skills),
|
||||
"tools": list(self.tools),
|
||||
"tool_approvals": dict(self.tool_approvals),
|
||||
}
|
||||
|
||||
|
||||
def effective_tool_approvals(tool_approvals, host_approval):
|
||||
"""What host.py will actually answer per declared tool, today.
|
||||
|
||||
``host_approval`` is the host subprocess's global approval
|
||||
(``"auto"`` or anything else, default ``"deny"``): host
|
||||
accepts an elicitation only when it is exactly ``"auto"``.
|
||||
Returns {tool: {"declared": policy, "effective": "accept" /
|
||||
"decline", "ask_honoured": bool}} -- ``ask_honoured`` is
|
||||
always False, because there is no ask channel at runtime:
|
||||
a declared ``ask`` currently declines, it does not ask.
|
||||
Reporting that plainly is the point of this function; a
|
||||
caller that presented ``ask`` as "will ask" would be lying.
|
||||
Unknown tools in the input are ignored (parse already
|
||||
rejects them at creation; this function is defensive).
|
||||
"""
|
||||
out = {}
|
||||
host_auto = host_approval == "auto"
|
||||
for tool, policy in (tool_approvals or {}).items():
|
||||
if tool not in KNOWN_TOOLS or policy not in TOOL_APPROVAL_POLICIES:
|
||||
continue
|
||||
if policy == "auto" and host_auto:
|
||||
out[tool] = {"declared": policy, "effective": "accept",
|
||||
"ask_honoured": False}
|
||||
else:
|
||||
out[tool] = {"declared": policy, "effective": "decline",
|
||||
"ask_honoured": False}
|
||||
return out
|
||||
|
||||
|
||||
def missing_required_tools(tools, config_text):
|
||||
"""Tools declared but with no [mcp_servers.<tool>] section.
|
||||
|
||||
@@ -240,6 +311,27 @@ def parse_agent_definition(data, known_skills=None):
|
||||
if unknown_tools:
|
||||
raise ValueError("agent.tools unknown tool(s): %s"
|
||||
% ", ".join(unknown_tools))
|
||||
tool_approvals = {}
|
||||
if data.get("tool_approvals") is not None:
|
||||
raw_approvals = data.get("tool_approvals")
|
||||
if not isinstance(raw_approvals, dict):
|
||||
raise ValueError("agent.tool_approvals must be an object")
|
||||
for tool, policy in raw_approvals.items():
|
||||
tool = _clean_str(tool, "agent.tool_approvals tool")
|
||||
if tool not in KNOWN_TOOLS:
|
||||
raise ValueError(
|
||||
"agent.tool_approvals unknown tool(s): %s" % tool)
|
||||
if policy not in TOOL_APPROVAL_POLICIES:
|
||||
raise ValueError(
|
||||
"agent.tool_approvals[%s] must be ask, auto or deny"
|
||||
% tool)
|
||||
tool_approvals[tool] = policy
|
||||
denied_required = [t for t in tools
|
||||
if tool_approvals.get(t) == "deny"]
|
||||
if denied_required:
|
||||
raise ValueError(
|
||||
"agent.tool_approvals denies required tool(s): %s"
|
||||
% ", ".join(denied_required))
|
||||
defaults = {}
|
||||
if data.get("defaults") is not None:
|
||||
raw_defaults = data.get("defaults")
|
||||
@@ -280,7 +372,8 @@ def parse_agent_definition(data, known_skills=None):
|
||||
return AgentDefinition(name=name, version=version,
|
||||
description=description,
|
||||
instructions=instructions, skills=skills,
|
||||
tools=tools, defaults=defaults)
|
||||
tools=tools, tool_approvals=tool_approvals,
|
||||
defaults=defaults)
|
||||
|
||||
|
||||
def load_agent_definition(path, agents_dir=None):
|
||||
@@ -378,6 +471,11 @@ def resolve_registry_agent(name, version=None, agents_dir=None,
|
||||
"instructions": definition.instructions,
|
||||
"skills": definition.skills,
|
||||
"tools": definition.tools,
|
||||
# v9: the re-parse must carry tool_approvals, or
|
||||
# a registry definition's declaration would be
|
||||
# silently dropped on the server path (inline
|
||||
# definitions never re-parse, so they were fine).
|
||||
"tool_approvals": definition.tool_approvals,
|
||||
"defaults": definition.defaults,
|
||||
},
|
||||
known_skills=known_skills)
|
||||
|
||||
Reference in new issue
Block a user