host
7f60f12639
resolve_registry_agent in cloud/agent_def.py: agent_name may be <name> or <name>@<version>, agent_version may also be given separately; both given must agree or 400. Unpinned loads <name>.json (default); pinned loads <name>.<version>.json and requires the file's own version field to match, with no silent fallback to the default file. Version strings are restricted to alphanumerics/._- (no path separators, no ..); the agents-dir traversal guard is a second line of defence. Server: agent_version is only for agent_name (inline agent with agent_version is 400); stored agent_name is the base name, not the @ form. Snapshot fixture: agents/research-brief.v1.json (same content as the default v1 file). Tests: test_agent_version.py (7 tests).
429 lines
19 KiB
Python
429 lines
19 KiB
Python
"""Declarative agent definitions (harness alignment, v1).
|
|
|
|
Zero third-party dependencies; standard library only.
|
|
|
|
A definition is a versioned JSON document that bundles the fields a
|
|
task already understands -- model, effort, approval, executor, plan --
|
|
plus a human-written instruction block and a list of required skills.
|
|
It is the Foundry-style "agent defined declaratively, harness runs
|
|
the loop" idea, cut down to what this codebase can actually honour
|
|
today, with every boundary written down instead of implied:
|
|
|
|
* Resolution happens at task creation (``POST /tasks`` with an inline
|
|
``agent`` object). Per field, an explicit body field wins over the
|
|
definition; a field absent from both stays NULL and follows the
|
|
existing worker-default chain. A definition that sets ``model``
|
|
therefore makes the task explicit for rule routing v0, and a
|
|
definition that leaves ``model`` empty leaves routing to the worker.
|
|
* Instructions reach the model only as a marked header prepended to
|
|
the stored prompt (``[Agent: <name>]``), because the dispatch path
|
|
(``cloud/sandbox.py`` -> ``host.py dispatch``) has no
|
|
developerInstructions channel today. This is NOT claimed to be
|
|
``developerInstructions``: it is visible in the stored prompt, in
|
|
the creation response, and in every later prompt read. Instructions
|
|
are rejected with goal tasks and coordinator tasks, where the
|
|
prompt is a goal or is never run as a model turn, so the header
|
|
cannot silently pollute a goal text or a coordinator parent.
|
|
* Skills are validated against the ``cloud/skills/`` pack at parse
|
|
time (an unknown skill is a 400, never a silent drop). Seeding
|
|
itself stays worker-global (``--seed-skills`` seeds the whole pack
|
|
into the tenant codex-home); v1 does NOT claim per-agent seeding.
|
|
When seeding is off, a listed skill is recorded in the creation
|
|
response but is not guaranteed present at run time -- that is the
|
|
documented v1 boundary, not a bug to be discovered later.
|
|
* Routing policy is deliberately NOT part of a definition in v1:
|
|
routing stays a worker-global policy (see ``cloud/models.py``).
|
|
Putting a routing object inside a definition would either be
|
|
silently dropped (a lie) or need a per-task policy column that does
|
|
not exist. A definition that contains ``routing`` / ``routing_policy``
|
|
is rejected, so a typo or a v2 idea cannot masquerade as v1 behaviour.
|
|
* Unknown top-level or defaults keys are rejected for the same
|
|
reason: a declarative file must fail on typos, not silently ignore
|
|
them.
|
|
|
|
v2 (this file, plus server/worker/sandbox/host changes) closes the
|
|
three v1 boundaries that were explicitly left open:
|
|
|
|
* Registry: definitions can live as files under repo-root ``agents/``
|
|
(``default_agents_dir()``) and be referenced by name
|
|
(``agent_name``) instead of only inline. The server never loads a
|
|
definition from a caller-supplied path; it only loads
|
|
``<name>.json`` under the registry dir, with the traversal guard
|
|
in ``load_agent_definition``.
|
|
* Persistence: ``tasks`` gains ``agent_name`` / ``agent_version`` /
|
|
``agent_instructions`` columns (see ``migrate_tasks_table``), so
|
|
``GET /tasks`` shows the definition identity and instructions, and
|
|
the worker no longer has to reconstruct them from a prompt header.
|
|
* Real channel: for local/docker executors, instructions travel as
|
|
``--developer-instructions`` on the host.py top-level command,
|
|
through ``sandbox`` into ``host.py dispatch`` ->
|
|
``create_thread(developerInstructions=...)``. The stored prompt
|
|
stays clean. For the generic cloud executor, which has no such
|
|
channel, the worker falls back to the v1 marked header in the
|
|
in-memory dispatch prompt only (DB prompt stays clean) -- that
|
|
fallback is the documented cloud boundary, not a claim that the
|
|
cloud runner honours developerInstructions. Goal and coordinator
|
|
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
|
|
import os
|
|
import re
|
|
from pathlib import Path
|
|
|
|
MAX_INSTRUCTIONS_CHARS = 8000
|
|
MAX_DESCRIPTION_CHARS = 1000
|
|
MAX_SKILLS = 20
|
|
NAME_RE = re.compile(r"^[a-z0-9][a-z0-9_-]{0,63}$")
|
|
EFFORTS = ("low", "medium", "high", "xhigh", "max")
|
|
APPROVALS = ("ask", "auto", "full")
|
|
EXECUTORS = ("local", "docker", "cloud")
|
|
_TOP_LEVEL_KEYS = ("name", "version", "description", "instructions",
|
|
"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"
|
|
|
|
|
|
def default_agents_dir():
|
|
"""Repo-root ``agents/`` directory (definitions live with code)."""
|
|
return AGENTS_DIR
|
|
|
|
|
|
def list_known_skills(skills_dir=None):
|
|
"""Sorted skill names present in the pack (dirs with SKILL.md)."""
|
|
base = Path(skills_dir) if skills_dir else SKILLS_DIR
|
|
names = []
|
|
try:
|
|
entries = sorted(base.iterdir())
|
|
except OSError:
|
|
return names
|
|
for entry in entries:
|
|
if entry.is_dir() and (entry / "SKILL.md").is_file():
|
|
names.append(entry.name)
|
|
return names
|
|
|
|
|
|
def _clean_str(value, field):
|
|
if not isinstance(value, str) or not value.strip():
|
|
raise ValueError("%s must be a non-empty string" % field)
|
|
return value.strip()
|
|
|
|
|
|
class AgentDefinition:
|
|
"""A parsed, validated agent definition."""
|
|
|
|
def __init__(self, name, version=None, description=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):
|
|
"""The identity echoed in the task-creation response."""
|
|
return {
|
|
"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.
|
|
|
|
``known_skills`` is a list of pack skill names; when it is not
|
|
None, every listed skill must be in it. Passing None skips the
|
|
existence check (pure-function tests), it never invents a skill.
|
|
"""
|
|
if not isinstance(data, dict):
|
|
raise ValueError("agent must be an object")
|
|
unknown = sorted(set(data) - set(_TOP_LEVEL_KEYS))
|
|
if unknown:
|
|
raise ValueError("agent has unknown field(s): %s"
|
|
% ", ".join(unknown))
|
|
name = data.get("name")
|
|
if not isinstance(name, str) or not NAME_RE.match(name):
|
|
raise ValueError(
|
|
"agent.name must match [a-z0-9][a-z0-9_-]{0,63}")
|
|
version = None
|
|
if data.get("version") is not None:
|
|
version = _clean_str(data.get("version"), "agent.version")
|
|
if len(version) > 64:
|
|
raise ValueError("agent.version is too long")
|
|
description = None
|
|
if data.get("description") is not None:
|
|
if not isinstance(data.get("description"), str):
|
|
raise ValueError("agent.description must be a string")
|
|
description = data["description"]
|
|
if len(description) > MAX_DESCRIPTION_CHARS:
|
|
raise ValueError("agent.description is too long")
|
|
instructions = None
|
|
if data.get("instructions") is not None:
|
|
if not isinstance(data.get("instructions"), str):
|
|
raise ValueError("agent.instructions must be a string")
|
|
instructions = data["instructions"].strip()
|
|
if not instructions:
|
|
instructions = None
|
|
elif len(instructions) > MAX_INSTRUCTIONS_CHARS:
|
|
raise ValueError(
|
|
"agent.instructions exceeds %d characters"
|
|
% MAX_INSTRUCTIONS_CHARS)
|
|
skills = []
|
|
if data.get("skills") is not None:
|
|
raw_skills = data.get("skills")
|
|
if not isinstance(raw_skills, list):
|
|
raise ValueError("agent.skills must be a list")
|
|
if len(raw_skills) > MAX_SKILLS:
|
|
raise ValueError("agent.skills has too many entries")
|
|
for skill in raw_skills:
|
|
skill = _clean_str(skill, "agent.skills entry")
|
|
if skill not in skills:
|
|
skills.append(skill)
|
|
if known_skills is not None:
|
|
known = set(known_skills)
|
|
missing = [s for s in skills if s not in known]
|
|
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")
|
|
if not isinstance(raw_defaults, dict):
|
|
raise ValueError("agent.defaults must be an object")
|
|
unknown_defaults = sorted(set(raw_defaults) - set(_DEFAULTS_KEYS))
|
|
if unknown_defaults:
|
|
raise ValueError("agent.defaults has unknown field(s): %s"
|
|
% ", ".join(unknown_defaults))
|
|
if raw_defaults.get("model") is not None:
|
|
defaults["model"] = _clean_str(raw_defaults["model"],
|
|
"agent.defaults.model")
|
|
if raw_defaults.get("effort") is not None:
|
|
if raw_defaults["effort"] not in EFFORTS:
|
|
raise ValueError(
|
|
"agent.defaults.effort must be low, medium, high,"
|
|
" xhigh or max")
|
|
defaults["effort"] = raw_defaults["effort"]
|
|
if raw_defaults.get("approval") is not None:
|
|
if raw_defaults["approval"] not in APPROVALS:
|
|
raise ValueError(
|
|
"agent.defaults.approval must be ask, auto or full")
|
|
defaults["approval"] = raw_defaults["approval"]
|
|
if raw_defaults.get("executor") is not None:
|
|
if raw_defaults["executor"] not in EXECUTORS:
|
|
raise ValueError(
|
|
"agent.defaults.executor must be local, docker"
|
|
" or cloud")
|
|
defaults["executor"] = raw_defaults["executor"]
|
|
if raw_defaults.get("plan") is not None:
|
|
plan = raw_defaults["plan"]
|
|
if plan not in (True, False, 0, 1):
|
|
raise ValueError("agent.defaults.plan must be true/false")
|
|
defaults["plan"] = 1 if plan in (True, 1) else 0
|
|
if defaults.get("plan") and defaults.get("approval") == "full":
|
|
raise ValueError(
|
|
"plan 模式与 approval=full 冲突（只读 vs 完全访问）")
|
|
return AgentDefinition(name=name, version=version,
|
|
description=description,
|
|
instructions=instructions, skills=skills,
|
|
tools=tools, defaults=defaults)
|
|
|
|
|
|
def load_agent_definition(path, agents_dir=None):
|
|
"""Load one definition from a JSON file (operator/local use only).
|
|
|
|
This is NOT an API path: the server never loads a definition from
|
|
a caller-supplied path (that would be a file-read primitive). When
|
|
``agents_dir`` is given, the resolved path must stay under it, so
|
|
``../`` cannot escape the definitions directory.
|
|
"""
|
|
candidate = Path(path)
|
|
if agents_dir is not None and not candidate.is_absolute():
|
|
candidate = Path(agents_dir) / candidate
|
|
resolved = candidate.resolve()
|
|
if agents_dir is not None:
|
|
base = Path(agents_dir).resolve()
|
|
if base not in resolved.parents and resolved != base:
|
|
raise ValueError("agent definition path escapes agents dir")
|
|
try:
|
|
with open(resolved, encoding="utf-8") as f:
|
|
data = json.load(f)
|
|
except OSError as exc:
|
|
raise ValueError("cannot read agent definition: %s" % exc)
|
|
except ValueError as exc:
|
|
raise ValueError("agent definition is not valid JSON: %s" % exc)
|
|
return parse_agent_definition(data)
|
|
|
|
|
|
VERSION_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$")
|
|
|
|
|
|
def resolve_registry_agent(name, version=None, agents_dir=None,
|
|
known_skills=None):
|
|
"""Resolve a registry definition, with optional version pinning.
|
|
|
|
``name`` may be ``<name>`` or ``<name>@<version>``; ``version``
|
|
may also be given separately. If both are given they must agree,
|
|
otherwise this is a 400 (ambiguity is never guessed). Without a
|
|
version, this loads ``<name>.json`` (the default/latest file).
|
|
With a version, it loads ``<name>.<version>.json`` and requires
|
|
the file's own ``version`` field to equal the requested version:
|
|
a misnamed snapshot (file says v1, filename says v2) is rejected
|
|
instead of being used as the wrong version, and a missing snapshot
|
|
is an error, never a silent fallback to the default file (that
|
|
fallback would defeat pinning entirely). A version string is
|
|
restricted to alphanumerics, ``.``, ``_``, ``-`` so it cannot
|
|
carry a path separator or ``..``; the agents-dir traversal guard
|
|
in load_agent_definition is a second line of defence, not the only
|
|
one. Skill existence is checked only when ``known_skills`` is
|
|
given (the server passes the pack; pure-function callers may omit
|
|
it, exactly as with parse_agent_definition).
|
|
"""
|
|
if not isinstance(name, str) or not name.strip():
|
|
raise ValueError("agent_name must be a non-empty string")
|
|
name = name.strip()
|
|
pinned_from_name = None
|
|
if "@" in name:
|
|
parts = name.split("@")
|
|
if len(parts) != 2 or not parts[0] or not parts[1]:
|
|
raise ValueError(
|
|
"agent_name with @ must be <name>@<version>")
|
|
name, pinned_from_name = parts
|
|
if version is not None:
|
|
if not isinstance(version, str) or not version.strip():
|
|
raise ValueError("agent_version must be a non-empty string")
|
|
version = version.strip()
|
|
if pinned_from_name is not None:
|
|
if version is not None and version != pinned_from_name:
|
|
raise ValueError(
|
|
"agent_version %r does not match agent_name version "
|
|
"%r" % (version, pinned_from_name))
|
|
version = pinned_from_name
|
|
if not NAME_RE.match(name):
|
|
raise ValueError(
|
|
"agent_name must match [a-z0-9][a-z0-9_-]{0,63}")
|
|
if version is not None and not VERSION_RE.match(version):
|
|
raise ValueError(
|
|
"agent_version must match [A-Za-z0-9][A-Za-z0-9._-]{0,63}")
|
|
filename = "%s.json" % name if version is None else \
|
|
"%s.%s.json" % (name, version)
|
|
definition = load_agent_definition(filename, agents_dir=agents_dir)
|
|
if version is not None and definition.version != version:
|
|
raise ValueError(
|
|
"registry file %s says version %r, not requested %r" %
|
|
(filename, definition.version, version))
|
|
if known_skills is not None:
|
|
# Re-parse for skill existence only (load already validated
|
|
# format); the re-parsed definition is the one returned, so
|
|
# callers never see a definition that failed skill checks.
|
|
definition = parse_agent_definition(
|
|
{
|
|
"name": definition.name,
|
|
"version": definition.version,
|
|
"description": definition.description,
|
|
"instructions": definition.instructions,
|
|
"skills": definition.skills,
|
|
"tools": definition.tools,
|
|
"defaults": definition.defaults,
|
|
},
|
|
known_skills=known_skills)
|
|
return definition
|
|
|
|
|
|
def resolve_task_fields(definition, body):
|
|
"""Resolve task fields: explicit body field wins, else definition.
|
|
|
|
``body`` is the task-creation body dict. Returns a dict with
|
|
``model``, ``effort``, ``approval``, ``executor`` (None when
|
|
absent from both) and ``plan`` (int 0/1). It never mutates either
|
|
input, and it never validates the resolved values -- the caller
|
|
(server task creation) runs its existing validation on them, so
|
|
a bad definition value is caught by the same 400 path as a bad
|
|
body value.
|
|
"""
|
|
body = body if isinstance(body, dict) else {}
|
|
defaults = definition.defaults if definition else {}
|
|
resolved = {}
|
|
for field in ("model", "effort", "approval", "executor"):
|
|
value = body.get(field)
|
|
if value is None:
|
|
value = defaults.get(field)
|
|
resolved[field] = value
|
|
if "plan" in body:
|
|
resolved["plan"] = 1 if body.get("plan") in (True, 1) else 0
|
|
elif defaults.get("plan") is not None:
|
|
resolved["plan"] = defaults["plan"]
|
|
else:
|
|
resolved["plan"] = 0
|
|
return resolved
|
|
|
|
|
|
def format_instructions_header(definition, prompt):
|
|
"""Prepend a definition's instructions to a prompt, marked.
|
|
|
|
Returns the original prompt unchanged when there is no definition
|
|
or no instructions, so callers that do not use definitions pay
|
|
nothing and see no header.
|
|
"""
|
|
if definition is None or not definition.instructions:
|
|
return prompt
|
|
header = "[Agent: %s" % definition.name
|
|
if definition.version:
|
|
header += " %s" % definition.version
|
|
header += "]"
|
|
return "%s\n%s\n\n%s" % (header, definition.instructions, prompt)
|