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
17 changes: 16 additions & 1 deletion .agents/skills/agent-release-gate/resources/qa_probe.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,21 @@ def resolve_credentials(env_file: str | pathlib.Path | None = None) -> None:
KEY = resolved["AGENTA_API_KEY"]


def default_tools(harness: str) -> list:
"""The shipped default grant list for `harness`.

On Pi this is Pi's default active built-ins, matching the shipped default agent template: an
empty list means "grant nothing" to the runner, so seeding `[]` would gate the release on a
configuration no real agent uses (issue #5590). Only Pi reads `builtin` grants; a Claude cell
brings its own tools, so seeding Pi's names there would test nothing.
"""
if not harness.startswith("pi"):
return []
return [
{"type": "builtin", "name": name} for name in ("read", "bash", "edit", "write")
]


def agent_template(harness: str, sandbox: str, model: str, provider: str) -> dict:
return {
"instructions": {"agents_md": "Be terse. Do exactly what is asked."},
Expand All @@ -79,7 +94,7 @@ def agent_template(harness: str, sandbox: str, model: str, provider: str) -> dic
"connection": {"mode": "agenta", "slug": None},
"extras": {},
},
"tools": [],
"tools": default_tools(harness),
"mcps": [],
"skills": [],
"harness": {"kind": harness},
Expand Down
17 changes: 16 additions & 1 deletion .agents/skills/agent-release-gate/resources/qa_product.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,21 @@ def outcome_for_input(t: "Turn", wanted_input: dict) -> str | None:
return None


def default_tools(harness: str) -> list:
"""The shipped default grant list for `harness`.

On Pi this is Pi's default active built-ins, matching the shipped default agent template: an
empty list means "grant nothing" to the runner, so seeding `[]` would gate the release on a
configuration no real agent uses (issue #5590). Only Pi reads `builtin` grants; a Claude cell
brings its own tools, so seeding Pi's names there would test nothing.
"""
if not harness.startswith("pi"):
return []
return [
{"type": "builtin", "name": name} for name in ("read", "bash", "edit", "write")
]


def template(
cell: dict,
tools: list | None = None,
Expand Down Expand Up @@ -749,7 +764,7 @@ def j5_commit(cell: dict) -> dict:
"agent": {
"instructions": {"agents_md": "seed"},
"llm": {"model": cell["model"], "provider": cell["provider"]},
"tools": [],
"tools": default_tools(cell["harness"]),
"harness": {"kind": cell["harness"]},
"sandbox": {"kind": cell["sandbox"]},
}
Expand Down
32 changes: 31 additions & 1 deletion api/oss/tests/pytest/unit/resources/test_workflow_catalog.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
from oss.src.resources.workflows.catalog import get_workflow_catalog_preset
from agenta.sdk.agents.pi_builtins import PI_DEFAULT_ACTIVE_BUILTINS
from agenta.sdk.engines.running.catalog import get_all_catalog_templates

from oss.src.resources.workflows.catalog import (
_build_template_data,
get_workflow_catalog_preset,
)


def test_feedback_quality_rating_preset_is_preserved_from_sdk_catalog():
Expand All @@ -14,3 +20,27 @@ def test_feedback_quality_rating_preset_is_preserved_from_sdk_catalog():
assert preset.data.uri == "agenta:custom:feedback:v0"
assert preset.data.schemas is not None
assert preset.data.schemas.outputs is not None


def test_agent_template_data_materializes_the_default_builtin_tools():
"""The agent template a new agent is created from must carry Pi's built-ins (issue #5590).

`_build_template_data` hoists the `agent` property's JSON-Schema default into a materialized
`data["parameters"]` block, then strips that non-primitive default back off the schema. That
hoist is the only thing that carries the shipped default into a newly created agent's
parameters, and the strip means the schema can no longer be the fallback source. Without this
test, an empty or dropped `tools` list would pass every builder-level test and still reach the
runner as "grant nothing", leaving saved agents with no read, bash, edit or write.
"""
expected_tools = [
{"type": "builtin", "name": name} for name in PI_DEFAULT_ACTIVE_BUILTINS
]

entry = next(
entry for entry in get_all_catalog_templates() if entry["key"] == "agent"
)
data = _build_template_data(entry["data"], settings_template=None)

assert data is not None
assert data["parameters"]["agent"]["tools"] == expected_tools
assert "default" not in data["schemas"]["parameters"]["properties"]["agent"]
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ Legend: (a) catalog/schema, (b) SDK neutral config, (c) runtime.
| Field | (a) schema | (b) SDK config | (c) runtime | Status |
| --- | --- | --- | --- | --- |
| model / provider | yes, `model: str` | yes, `Optional[str]` | wired to the runner | Loose string. No `ModelRef`, no provider enum. There is no separate provider field. |
| tools | yes, strict list | yes, lenient coercion | wired, resolved to builtin names + tool specs | Entries strict, list lenient. |
| tools | yes, strict list | yes, lenient coercion | wired, resolved to builtin names + tool specs | Entries strict, list lenient. The shipped default template fills it with Pi's four default built-ins (`read`, `bash`, `edit`, `write`); see [Tools](tools.md). |
| mcp_servers | yes, strict list | yes | wired, resolved to runner MCP servers | Strict per entry. Claude supports external HTTP servers; Pi refuses them until its bridge exists. |
| skills | yes, embed/inline list | yes | wired | Author-settable (`SkillConfig` inline or `@ag.embed` references). The playground build-kit overlay embeds one skill, the `build-an-agent` playbook; the `pi_agenta` harness additionally force-unions `getting-started`. See below. |
| persona | no | no | wired but forced only | Not a config field. The Agenta harness hardcodes an append-system preamble. See below. |
Expand All @@ -238,7 +238,8 @@ Pi (`pi_core`) and Claude harnesses get no forced skills or persona.

Per-harness divergence is real in other ways, but not in permission enforcement anymore: the
permission policy is now enforced on both Claude and Pi. Builtin tool names are dropped for
Claude with a warning, because builtins are Pi-only. Forced skills and persona are
Claude, because builtins are Pi-only; that drop warns only when the set differs from Pi's four
defaults, which is the set the shipped template carries. Forced skills and persona are
Agenta-only. Pi's
`system` and `append_system` overrides come through the `harness_kwargs` escape hatch on the
neutral config, which is itself absent from the schema.
Expand Down
26 changes: 25 additions & 1 deletion docs/design/agent-workflows/documentation/tools.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ shares two fields through `ToolConfigBase`, and then a `type` discriminator pick

| Config (`type`) | Carries | Example use |
| --- | --- | --- |
| `builtin` | `name` | A harness-native tool such as Pi's `read` or `web_search`. |
| `builtin` | `name` | A harness-native tool such as Pi's `read` or `web_search`. The shipped default template grants Pi's four default built-ins (`read`, `bash`, `edit`, `write`); see [the grant list](#built-in-tools-the-harness-runs-them-natively-gated-through-the-same-relay). |
| `gateway` | `provider`, `integration`, `action`, `connection`, optional `name` | A Composio action, like `github__create_issue` on a connected account. |
| `code` | `name`, `runtime` (`python`/`node`), `script`, `input_schema`, `secrets` | An inline snippet the author writes, with named vault secrets injected. |
| `client` | `name`, `input_schema` | A tool the browser fulfils, like "ask the user to pick a date." |
Expand Down Expand Up @@ -401,6 +401,30 @@ separately, at session start. The extension edits Pi's active tool set at
every non-builtin tool untouched. A builtin outside the grant list is simply absent from the
model's active tools, so no call for it ever fires, and the permission hook never sees it.

Three values of the wire `tools` field mean three different things, and the difference is the
whole reason this list exists:

- **Omitted.** The runner falls back to Pi's own default active set, `read`, `bash`, `edit`,
`write` (`PI_DEFAULT_ACTIVE_BUILTINS` in `run-plan.ts`).
- **`[]`.** Grant nothing. Every builtin is removed from the model's active tools. This is an
author saying "no builtins", not an author saying nothing.
- **A list of names.** Grant exactly those.

The shipped default agent template names Pi's four defaults explicitly rather than leaving the
field empty, so a saved agent carries the same builtins Pi would have activated on its own. The
Python side of that list is `PI_DEFAULT_ACTIVE_BUILTINS` in
`sdks/python/agenta/sdk/agents/pi_builtins.py`; it and the runner's TypeScript constant are two
implementations of one contract, pinned against the shared golden fixture
`sdks/python/oss/tests/pytest/unit/agents/golden/pi_default_active_builtins.json`.

Granting a builtin is not the same as allowing it to run. Under the default permission mode
`allow_reads`, a default agent runs `read` without asking and raises an approval on every
`bash`, `edit`, and `write` call.

Agents saved before that default shipped still carry `tools: []` and therefore still have no
builtins. Nothing repairs them automatically. Their author has to open the agent, expand
Advanced, select the built-ins in the "Built-in tools" control, and commit a new revision.

### External MCP servers: remote HTTP connections

A declared user MCP server contains identity, an HTTP connection, credential references, and
Expand Down
4 changes: 2 additions & 2 deletions docs/design/agent-workflows/interfaces/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ page. `Status` is read from each page's prose: **stable** (wired and unlikely to
| [`/invoke`](public-edge/workflow-invoke.md) | public | `decorators/routing.py`, `models/workflows.py`, `agent/app.py` | stable | `unit/agent/`, `utils/test_messages_endpoint.py` |
| [`/inspect`](public-edge/workflow-inspect.md) | public | `agent/schemas.py`, `agent/app.py` (builtin-URI binding), `models/workflows.py`, `decorators/routing.py` | stable | `unit/agents/test_dtos_agent_config.py`, `unit/agent/test_builtin_uri_binding.py` |
| [`/messages`](public-edge/agent-messages.md) | public | `adapters/vercel/{routing,messages,stream}.py`, `agentRequest.ts` | evolving (create-or-resume not observable until storage lands) | `utils/test_messages_endpoint.py`, `unit/agents/test_ui_messages.py` |
| [Agent config schema](public-edge/agent-config-schema.md) | public | `agent/schemas.py`, `sdk/utils/types.py`, `agents/dtos.py` (`HARNESS_IDENTITIES`) | stable | `unit/agents/test_dtos_agent_config.py`, `unit/agents/test_harness_identity.py` |
| [Agent config schema](public-edge/agent-config-schema.md) | public | `agent/schemas.py`, `sdk/utils/types.py`, `agents/dtos.py` (`HARNESS_IDENTITIES`), `sdk/agents/pi_builtins.py` (`PI_DEFAULT_ACTIVE_BUILTINS`) | stable | `unit/agents/test_dtos_agent_config.py`, `unit/agents/test_harness_identity.py`, `unit/agents/test_pi_builtins_parity.py` + `golden/pi_default_active_builtins.json`, `services/oss/tests/pytest/unit/agent/test_default_agent_template.py` |
| [`/run`](cross-service/service-to-agent-runner.md) | cross-service (the spine) | `protocol.ts`, `utils/wire.py`, `utils/ts_runner.py`, `server.ts`/`cli.ts` | stable (pinned by golden) | `unit/agents/test_wire_contract.py` + `golden/`, `services/agent/tests/unit/wire-contract.test.ts` |
| [Runner to harness](cross-service/runner-to-harness.md) | cross-service (ACP) | `engines/sandbox_agent.ts` + `sandbox_agent/{run-plan,capabilities,permissions}.ts` | evolving | `services/agent/tests/unit/sandbox-agent-*.test.ts` |
| [Runner to MCP server](cross-service/runner-to-mcp-server.md) | cross-service | `agents/mcp/`, `engines/sandbox_agent/{mcp,tool-mcp-assets,relay-guard}.ts`, `tools/{mcp-bridge,tool-mcp-http,tool-mcp-stdio,tool-mcp-env,relay,relay-client,relay-protocol,relay-watch}.ts` | evolving (internal channel delivered locally over loopback HTTP and on Daytona via the in-sandbox stdio shim, `client` tools included — a client call parks via a paused relay answer; user stdio disabled) | `services/runner/tests/unit/{mcp-servers,session-mcp-layering,tool-mcp-assets,tool-mcp-stdio,tool-relay-guard}.test.ts` |
Expand All @@ -55,7 +55,7 @@ page. `Status` is read from each page's prose: **stable** (wired and unlikely to
| [Neutral runtime DTOs](in-service/neutral-runtime-dtos.md) | in-service | `agents/dtos.py` | stable | `unit/agents/test_dtos_*.py`, `test_harness_identity.py` |
| [Runtime ports](in-service/runtime-ports.md) | in-service | `agents/interfaces.py` | evolving (`LocalBackend` stub) | `unit/agents/test_environment_lifecycle.py`, `test_harness_adapters.py` |
| [Backend adapter](in-service/backend-adapter.md) | in-service | `agents/adapters/sandbox_agent.py` | stable | `unit/agents/test_runner_adapter_config.py`, `test_environment_lifecycle.py` |
| [Harness adapters](in-service/harness-adapters.md) | in-service | `agents/adapters/harnesses.py`, `agents/adapters/claude_settings.py`, `agents/dtos.py` | stable | `unit/agents/test_harness_adapters.py`, `test_dtos_harness_configs.py`, `unit/agents/adapters/test_claude_settings.py` |
| [Harness adapters](in-service/harness-adapters.md) | in-service | `agents/adapters/harnesses.py`, `agents/adapters/claude_settings.py`, `agents/dtos.py`, `sdk/agents/pi_builtins.py` | stable | `unit/agents/test_harness_adapters.py`, `test_dtos_harness_configs.py`, `unit/agents/adapters/test_claude_settings.py` |
| [Browser protocol adapter](in-service/browser-protocol-adapter.md) | in-service | `agents/adapters/vercel/{routing,messages,stream,sse}.py` | stable | `unit/agents/test_ui_messages.py`, `utils/test_messages_endpoint.py` |
| [Tool models and resolution](in-service/tool-models-and-resolution.md) | in-service | `agents/tools/models.py`, `platform/{gateway,workflow,op_catalog,platform_tools}.py`, `agent/tools/resolver.py` | evolving | `unit/agents/tools/`, `unit/agents/platform/test_op_catalog.py` |
| [MCP models and resolution](in-service/mcp-models-and-resolution.md) | in-service | `agents/mcp/{models,resolver,wire}.py` | evolving (stdio wired; remote deferred; resolution feature-gated) | `unit/agents/mcp/` |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,12 @@ Each adapter implements `_to_harness_config(...)` and emits a different `/run` w
harnesses. Pi has no native permission gate of its own (no `.claude/settings.json` equivalent),
so the runner's tool relay enforces `permissions` for Pi at execution time; an `ask` verdict
pauses the run and Pi gets the same human-in-the-loop approval Claude gets at its gate.
- **`ClaudeHarness`** delivers tools over MCP, not natively, and has no Pi built-ins (it warns
if any are set). It carries `permissions` and renders `.claude/settings.json` from four
- **`ClaudeHarness`** delivers tools over MCP, not natively, and has no Pi built-ins: it drops
the names. It warns only when the set differs from `PI_DEFAULT_ACTIVE_BUILTINS`, because the
shipped default template carries exactly that set and warning on it would fire on nearly every
Claude run. A set the author touched (a subset, a superset, an unrelated name) still warns, and
the message names the dropped tools. It carries `permissions` and renders
`.claude/settings.json` from four
sources — the author's `harness_kwargs["claude"]["permissions"]` slice, the sandbox permission,
each user MCP server's permission (`mcp__<server>` rules), and each resolved EXECUTABLE tool's
permission (`mcp__agenta-tools__<name>` rules; F-046) — shipped as `harnessFiles`. It carries
Expand All @@ -45,13 +49,19 @@ The wire shapes, side by side:
- `sdks/python/agenta/sdk/agents/adapters/harnesses.py`: the three adapters.
- `sdks/python/agenta/sdk/agents/dtos.py`: the `PiAgentConfig`/`ClaudeAgentConfig`/
`AgentaAgentConfig` wire emitters.
- `sdks/python/agenta/sdk/agents/pi_builtins.py`: `PI_DEFAULT_ACTIVE_BUILTINS`, the set
`ClaudeHarness` stays silent about.

## Watch for when changing

- **Tool delivery per harness.** Native versus MCP is the load-bearing difference. Pi takes
tools natively; everyone else gets them over the MCP bridge.
- **Prompt override behavior.** Pi replaces or appends; Claude reads options; Agenta composes.
- **Forced Agenta behavior.** Instruction composition and the forced tool set are deliberate.
- **The Claude built-in warning predicate.** It is exact-set equality against
`PI_DEFAULT_ACTIVE_BUILTINS`, not a per-name filter. A per-name filter would silence an
authored subset such as `["bash"]`, which is exactly the case the warning exists for. If the
default template's built-in set changes, this predicate changes with it.
- **Claude skill delivery.** Claude wires inline skills like the other harnesses; the runner
materializes them under `.claude/skills`. (An earlier revision suppressed Claude's
`wire_skills()` to `{}`; that override is gone, and `test_claude_carries_skills_for_project_local_materialization`
Expand Down
Loading
Loading