Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,15 @@ descriptor undermines the framework.
- `AgentOutput` shape on every successful execution (fields may be `None`).
- Raise real exceptions on real errors — no silent failure swallowing.
- Declare accurate `ProviderCapabilities` matching observed behavior.
- Declare `skills` accurately. Skills are **not** an allowed carve-out — a
provider reaches `skills=True` either natively (`supports_native_skills=True`,
forwarding `skill_directories` to its SDK) or via `AgentExecutor`'s eager
preamble injection, which is provider-agnostic. Declare `False` only when
neither path can work (e.g. `aca`, where skill directories are host paths the
in-sandbox runner cannot read). `config/validator.py` cross-checks per-agent
`skills:` and inherited `runtime.skills` against this flag, so an inaccurate
`False` turns into a spurious validate error and an inaccurate `True` silently
drops the skill content at run time.
- Provide a smoke test that exercises construct + execute paths against
a mocked SDK.
- Maintain `concurrent_safe: true`, or fail validation when used in
Expand Down
41 changes: 41 additions & 0 deletions src/conductor/config/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,7 @@ def _validate_provider_capabilities(
runtime_default_effort = config.workflow.runtime.default_reasoning_effort
runtime_max_session_seconds = config.workflow.runtime.max_session_seconds
runtime_working_dir = config.workflow.runtime.working_dir
runtime_skills = config.workflow.runtime.skills

# Cache per provider name so we don't re-resolve for every agent.
cache: dict[str, ProviderCapabilities] = {}
Expand Down Expand Up @@ -1763,6 +1764,18 @@ def _check_agent_capabilities(
f"directories (capabilities.working_dir=False)."
)

# skills: a provider that cannot surface skill content would drop it
# silently — the agent still runs, just without the knowledge the
# author asked for. An empty list is an explicit opt-out, so only a
# non-empty list is an error.
if agent.skills and not caps.skills:
errors.append(
f"Agent '{agent.name}' declares skills={agent.skills!r} but provider "
f"'{provider_name}' does not support skills "
f"(capabilities.skills=False). Remove the skills, opt out with "
f"'skills: []', or override the agent to a skill-aware provider."
)

# All provider-backed agents that run at workflow scope: top-level agents
# PLUS for_each inline agents (``ForEachDef.agent``), which inherit the
# workflow-level ``mcp_servers`` / ``max_session_seconds`` and run with
Expand Down Expand Up @@ -1854,6 +1867,34 @@ def _check_agent_capabilities(
f"working_dir."
)

# ----- Workflow-level: skills -----
# A runtime-wide skills list is inherited by every LLM agent that does not
# declare its own (``skills: []`` is an explicit opt-out and counts as an
# override). A provider that cannot surface skill content would drop it
# silently, so error against every resolved provider that actually
# receives the setting.
if runtime_skills:
providers_inheriting_skills: dict[str, list[str]] = {}
for agent in all_llm_agents:
# Any per-agent ``skills:`` — including the empty-list opt-out —
# replaces the runtime default; that case is checked in
# ``_check_agent_capabilities`` instead.
if agent.skills is not None:
continue
pname = _resolved_provider_name(agent, default_provider)
providers_inheriting_skills.setdefault(pname, []).append(agent.name)
for pname, agent_names in providers_inheriting_skills.items():
pcaps = _caps_for(pname)
if pcaps is not None and not pcaps.skills:
errors.append(
f"Workflow declares 'runtime.skills'={sorted(runtime_skills)!r} "
f"but provider '{pname}' does not support skills "
f"(capabilities.skills=False) and is used by agent(s): "
f"{sorted(agent_names)!r}. Override these agents to a "
f"skill-aware provider, opt out per-agent with 'skills: []', "
f"or remove the workflow-level skills."
)

# ----- Per-agent checks -----
for agent in config.agents:
if not _is_llm_agent(agent):
Expand Down
149 changes: 149 additions & 0 deletions tests/test_config/test_validator_capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def _caps(**overrides: object) -> ProviderCapabilities:
"checkpoint_resume": True,
"usage_tracking": True,
"concurrent_safe": True,
"skills": True,
}
base.update(overrides)
return ProviderCapabilities(**base) # type: ignore[arg-type]
Expand All @@ -49,10 +50,13 @@ def _build_workflow(
for_each: list[ForEachDef] | None = None,
mcp_servers: dict[str, MCPServerDef] | None = None,
tools: list[str] | None = None,
skills: list[str] | None = None,
) -> WorkflowConfig:
runtime_kwargs: dict[str, Any] = {"provider": "copilot"}
if mcp_servers is not None:
runtime_kwargs["mcp_servers"] = mcp_servers
if skills is not None:
runtime_kwargs["skills"] = skills
workflow_kwargs: dict[str, Any] = {}
if tools is not None:
workflow_kwargs["tools"] = tools
Expand All @@ -74,6 +78,7 @@ def _for_each_workflow(
inline: AgentDef,
tools: list[str] | None = None,
mcp_servers: dict[str, MCPServerDef] | None = None,
skills: list[str] | None = None,
) -> WorkflowConfig:
"""Build a workflow whose only for_each group carries ``inline``.

Expand All @@ -86,6 +91,7 @@ def _for_each_workflow(
agents=[AgentDef(name="entry", prompt="hi", tools=[])],
tools=tools,
mcp_servers=mcp_servers,
skills=skills,
for_each=[
ForEachDef(
name="loop",
Expand Down Expand Up @@ -1559,3 +1565,146 @@ def test_non_empty_tools_is_still_rejected(self, patch_caps: Any) -> None:
config = self._sdk_workflow(agents=[AgentDef(name="a", prompt="hi", tools=["search"])])
with pytest.raises(ConfigurationError, match="does not honor per-agent tool allowlists"):
validate_workflow_config(config)


class TestSkillsCrossCheck:
"""Requirement: ``skills`` (per-agent or runtime-wide) against a provider
declaring ``skills=False`` is a hard validate error.

``capabilities.skills`` promises this cross-check in its docstring, but the
check was missing — a workflow on a skill-blind provider validated cleanly
and then silently dropped the skill content at run time (follow-up to #180).
"""

def test_agent_skills_against_unsupported_provider_errors(self, patch_caps: Any) -> None:
patch_caps({"copilot": _caps(skills=False)})
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi", skills=["conductor"])],
)
with pytest.raises(ConfigurationError, match="does not support skills"):
validate_workflow_config(config)

def test_agent_skills_against_supported_provider_passes(self, patch_caps: Any) -> None:
patch_caps({"copilot": _caps(skills=True)})
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi", skills=["conductor"])],
)
validate_workflow_config(config) # no raise

def test_empty_skills_list_is_opt_out_not_an_error(self, patch_caps: Any) -> None:
"""``skills: []`` asks for nothing, so a skill-blind provider is fine."""
patch_caps({"copilot": _caps(skills=False)})
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi", skills=[])],
)
validate_workflow_config(config) # no raise

def test_runtime_skills_against_unsupported_provider_errors(self, patch_caps: Any) -> None:
"""runtime.skills is inherited by every LLM agent that does not override."""
patch_caps({"copilot": _caps(skills=False)})
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi")],
skills=["conductor"],
)
with pytest.raises(ConfigurationError, match="runtime.skills"):
validate_workflow_config(config)

def test_runtime_skills_per_agent_opt_out_passes(self, patch_caps: Any) -> None:
"""A per-agent ``skills: []`` overrides the runtime default, so the
skill-blind provider never receives it."""
patch_caps({"copilot": _caps(skills=False)})
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi", skills=[])],
skills=["conductor"],
)
validate_workflow_config(config) # no raise

def test_runtime_skills_all_agents_override_to_capable_provider_passes(
self, patch_caps: Any
) -> None:
patch_caps(
{
"copilot": _caps(skills=False),
"claude": _caps(skills=True),
}
)
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi", provider="claude")],
skills=["conductor"],
)
validate_workflow_config(config) # no raise

def test_per_agent_provider_override_to_skill_blind_provider_errors(
self, patch_caps: Any
) -> None:
patch_caps(
{
"copilot": _caps(skills=True),
"claude": _caps(skills=False),
}
)
config = _build_workflow(
agents=[AgentDef(name="a", prompt="hi", provider="claude", skills=["conductor"])],
)
with pytest.raises(ConfigurationError, match="does not support skills"):
validate_workflow_config(config)

def test_for_each_inline_agent_skills_errors(self, patch_caps: Any) -> None:
"""A for_each inline agent is not in ``config.agents``; it must still be
cross-checked (#270 pattern)."""
patch_caps({"copilot": _caps(skills=False)})
config = _for_each_workflow(
inline=AgentDef(name="inline", prompt="hi", tools=[], skills=["conductor"]),
)
with pytest.raises(ConfigurationError, match="does not support skills"):
validate_workflow_config(config)

def test_for_each_inline_agent_inherits_runtime_skills_errors(self, patch_caps: Any) -> None:
"""The inline agent inherits runtime.skills just like a top-level agent."""
patch_caps({"copilot": _caps(skills=False)})
config = _for_each_workflow(
inline=AgentDef(name="inline", prompt="hi", tools=[]),
skills=["conductor"],
)
with pytest.raises(ConfigurationError, match="runtime.skills"):
validate_workflow_config(config)


class TestAcaSkillsRealCapabilities:
"""Cross-check against the REAL ``AcaRuntimeProvider`` descriptor, which
declares ``skills=False`` — skill directories are host paths the in-sandbox
runner cannot read. ``aca`` is workflow-level only (no ``AgentDef.provider``
literal), so it is set via ``runtime.provider``.
"""

def _aca_workflow(self, *, agents: list[AgentDef], skills: list[str] | None = None):
from conductor.config.schema import ProviderSettings

runtime_kwargs: dict[str, Any] = {
"provider": ProviderSettings(name="aca", pool_endpoint="https://pool.example.com"),
}
if skills is not None:
runtime_kwargs["skills"] = skills
return WorkflowConfig(
workflow=WorkflowDef(
name="test",
entry_point=agents[0].name,
runtime=RuntimeConfig(**runtime_kwargs),
),
agents=agents,
)

def test_real_aca_rejects_agent_skills(self) -> None:
config = self._aca_workflow(
agents=[AgentDef(name="a", prompt="hi", skills=["conductor"])],
)
with pytest.raises(ConfigurationError, match="does not support skills"):
validate_workflow_config(config)

def test_real_aca_rejects_runtime_skills(self) -> None:
config = self._aca_workflow(
agents=[AgentDef(name="a", prompt="hi")],
skills=["conductor"],
)
with pytest.raises(ConfigurationError, match="runtime.skills"):
validate_workflow_config(config)
Loading