feat: agent tools as required tools with tenant tool gate
Agent definitions gain tools (websearch/imagegen/playwright, unknown tool is a 400), stored as agent_tools JSON and returned by GET /tasks. This is deliberately required-tools, not an allowlist: create_thread has no tool allowlist and tenant homes are per-project shared, so blocking other tools cannot be claimed. Instead the worker fails fast before any model call when a declared tool has no [mcp_servers.<tool>] section in the tenant codex-home config.toml (checked after _tenant_dirs seeding, so a section seeded earlier by another worker counts as present). The gate runs only for tenant-isolated Docker workers; local/cloud workers skip it, documented, not verified. Tests: test_agent_tools.py (8 tests).
This commit is contained in:
1 parent
1672ab9ff2
commit
fd37505c48
6 files changed
+383
-7
No files matched your search
+53
-3
@@ -66,6 +66,27 @@ three v1 boundaries that were explicitly left open:
|
||||
tasks still reject instructions in v2: the goal loop and the
|
||||
coordinator parent do not pass instructions through, and silently
|
||||
dropping them there would be worse than rejecting them.
|
||||
|
||||
v3 (tools, this file plus server/worker changes) adds a ``tools``
|
||||
field, but deliberately as REQUIRED tools, not an allowlist:
|
||||
|
||||
* A definition may declare ``tools`` from the MCP servers this
|
||||
codebase actually seeds: ``websearch``, ``imagegen``,
|
||||
``playwright``. An unknown tool is a 400, never a silent drop.
|
||||
* The worker does NOT claim to block other tools. Blocking would
|
||||
need a tool allowlist in ``create_thread`` (which does not exist)
|
||||
or per-task codex-homes (tenant homes are per-project and shared,
|
||||
and MCP seeding is worker-global), so an allowlist claim would be
|
||||
false. What v3 does instead is fail fast on the opposite, silent
|
||||
failure: a task that declares a required tool but whose tenant
|
||||
codex-home config.toml has no ``[mcp_servers.<tool>]`` section
|
||||
at dispatch time is marked failed before any model call, with the
|
||||
missing tools named. The check runs only for tenant-isolated
|
||||
Docker workers, where the tenant config.toml is the actual
|
||||
availability record (worker flags alone would be wrong: a section
|
||||
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".
|
||||
"""
|
||||
|
||||
import json
|
||||
@@ -81,7 +102,8 @@ EFFORTS = ("low", "medium", "high", "xhigh", "max")
|
||||
APPROVALS = ("ask", "auto", "full")
|
||||
EXECUTORS = ("local", "docker", "cloud")
|
||||
_TOP_LEVEL_KEYS = ("name", "version", "description", "instructions",
|
||||
"skills", "defaults")
|
||||
"skills", "tools", "defaults")
|
||||
KNOWN_TOOLS = ("websearch", "imagegen", "playwright")
|
||||
_DEFAULTS_KEYS = ("model", "effort", "approval", "executor", "plan")
|
||||
SKILLS_DIR = Path(__file__).resolve().parent / "skills"
|
||||
AGENTS_DIR = Path(__file__).resolve().parent.parent / "agents"
|
||||
@@ -116,12 +138,14 @@ class AgentDefinition:
|
||||
"""A parsed, validated agent definition."""
|
||||
|
||||
def __init__(self, name, version=None, description=None,
|
||||
instructions=None, skills=None, defaults=None):
|
||||
instructions=None, skills=None, tools=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.defaults = dict(defaults or {})
|
||||
|
||||
def to_summary(self):
|
||||
@@ -130,9 +154,22 @@ class AgentDefinition:
|
||||
"name": self.name,
|
||||
"version": self.version,
|
||||
"skills": list(self.skills),
|
||||
"tools": list(self.tools),
|
||||
}
|
||||
|
||||
|
||||
def missing_required_tools(tools, config_text):
|
||||
"""Tools declared but with no [mcp_servers.<tool>] section.
|
||||
|
||||
Pure function over the tenant config.toml text. An empty or
|
||||
missing config means every declared tool is missing -- a task
|
||||
that needs a tool must not run on a home that never seeded it.
|
||||
"""
|
||||
text = config_text or ""
|
||||
return [t for t in (tools or [])
|
||||
if ("[mcp_servers.%s]" % t) not in text]
|
||||
|
||||
|
||||
def parse_agent_definition(data, known_skills=None):
|
||||
"""Parse and validate one definition, or raise ValueError.
|
||||
|
||||
@@ -190,6 +227,19 @@ def parse_agent_definition(data, known_skills=None):
|
||||
if missing:
|
||||
raise ValueError("agent.skills unknown skill(s): %s"
|
||||
% ", ".join(missing))
|
||||
tools = []
|
||||
if data.get("tools") is not None:
|
||||
raw_tools = data.get("tools")
|
||||
if not isinstance(raw_tools, list):
|
||||
raise ValueError("agent.tools must be a list")
|
||||
for tool in raw_tools:
|
||||
tool = _clean_str(tool, "agent.tools entry")
|
||||
if tool not in tools:
|
||||
tools.append(tool)
|
||||
unknown_tools = [t for t in tools if t not in KNOWN_TOOLS]
|
||||
if unknown_tools:
|
||||
raise ValueError("agent.tools unknown tool(s): %s"
|
||||
% ", ".join(unknown_tools))
|
||||
defaults = {}
|
||||
if data.get("defaults") is not None:
|
||||
raw_defaults = data.get("defaults")
|
||||
@@ -230,7 +280,7 @@ def parse_agent_definition(data, known_skills=None):
|
||||
return AgentDefinition(name=name, version=version,
|
||||
description=description,
|
||||
instructions=instructions, skills=skills,
|
||||
defaults=defaults)
|
||||
tools=tools, defaults=defaults)
|
||||
|
||||
|
||||
def load_agent_definition(path, agents_dir=None):
|
||||
|
||||
Reference in new issue
Block a user