fix(agents): Always activate Pi built-ins, control them with rules - #5651
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change removes Pi built-in tools from agent configuration. The runner always activates seven built-ins and uses ChangesAlways-active Pi built-ins
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant AgentConfig
participant SDK
participant RunPlan
participant PiExtension
participant PermissionPlan
AgentConfig->>SDK: provide tools and harness.permissions
SDK->>SDK: ignore legacy builtin entries
SDK-->>RunPlan: return custom tool specs and compatibility names
RunPlan->>PermissionPlan: evaluate allow, ask, and deny rules
PermissionPlan-->>RunPlan: return builtinGatingActive
RunPlan->>PiExtension: enable all seven Pi built-ins
PiExtension->>PermissionPlan: check builtin execution
PermissionPlan-->>PiExtension: allow, ask, or deny outcome
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 11
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e9b1f24-78ce-4db8-84e4-c30143de1b16
📒 Files selected for processing (81)
.agents/skills/agent-release-gate/resources/LESSONS.md.agents/skills/agent-release-gate/resources/coverage.md.agents/skills/agent-release-gate/resources/qa_longctx.py.agents/skills/agent-release-gate/resources/qa_probe.py.agents/skills/agent-release-gate/resources/qa_product.pyapi/oss/src/core/workflows/build_kit.pyapi/oss/tests/pytest/unit/applications/test_build_kit_overlay.pyapi/oss/tests/pytest/unit/resources/test_workflow_catalog.pydocs/design/agent-workflows/documentation/adapters/agenta.mddocs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mddocs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.mddocs/design/agent-workflows/projects/default-agent-builtins/design.mddocs/design/agent-workflows/projects/default-agent-builtins/status.mddocs/design/agent-workflows/projects/default-agent-builtins/testing.mddocs/design/agent-workflows/projects/pi-builtin-gating/status.mdsdks/python/agenta/sdk/agents/__init__.pysdks/python/agenta/sdk/agents/adapters/agenta_builtins.pysdks/python/agenta/sdk/agents/adapters/harnesses.pysdks/python/agenta/sdk/agents/dtos.pysdks/python/agenta/sdk/agents/handler.pysdks/python/agenta/sdk/agents/pi_builtins.pysdks/python/agenta/sdk/agents/tools/__init__.pysdks/python/agenta/sdk/agents/tools/errors.pysdks/python/agenta/sdk/agents/tools/models.pysdks/python/agenta/sdk/agents/tools/resolver.pysdks/python/agenta/sdk/utils/types.pysdks/python/oss/tests/pytest/integration/agents/_qa_transcripts.pysdks/python/oss/tests/pytest/unit/agents/golden/pi_builtin_tools.jsonsdks/python/oss/tests/pytest/unit/agents/golden/run_request.attachment.jsonsdks/python/oss/tests/pytest/unit/agents/golden/run_request.pi_core.jsonsdks/python/oss/tests/pytest/unit/agents/platform/test_resolve.pysdks/python/oss/tests/pytest/unit/agents/test_agenta_builtins_reference_files.pysdks/python/oss/tests/pytest/unit/agents/test_dtos_harness_configs.pysdks/python/oss/tests/pytest/unit/agents/test_harness_adapters.pysdks/python/oss/tests/pytest/unit/agents/test_pi_builtins_parity.pysdks/python/oss/tests/pytest/unit/agents/test_wire_contract.pysdks/python/oss/tests/pytest/unit/agents/tools/test_models.pysdks/python/oss/tests/pytest/unit/agents/tools/test_resolver.pyservices/oss/src/agent/config.pyservices/oss/tests/pytest/integration/agent/tools/test_gateway_http.pyservices/oss/tests/pytest/unit/agent/test_config_template_fallback.pyservices/oss/tests/pytest/unit/agent/test_default_agent_template.pyservices/oss/tests/pytest/unit/agent/test_invoke_handler.pyservices/oss/tests/pytest/unit/agent/tools/test_resolution.pyservices/runner/config/agent.jsonservices/runner/src/engines/sandbox_agent/environment-setup.tsservices/runner/src/engines/sandbox_agent/pi-assets.tsservices/runner/src/engines/sandbox_agent/run-plan.tsservices/runner/src/engines/sandbox_agent/session-identity.tsservices/runner/src/extensions/agenta.tsservices/runner/src/permission-plan.tsservices/runner/src/protocol.tsservices/runner/tests/unit/extension-tools.test.tsservices/runner/tests/unit/permission-plan.test.tsservices/runner/tests/unit/pi-builtin-activation.test.tsservices/runner/tests/unit/pi-builtin-tools-parity.test.tsservices/runner/tests/unit/sandbox-agent-pi-assets.test.tsservices/runner/tests/unit/sandbox-agent-qa-transcript-replay.test.tsservices/runner/tests/unit/sandbox-agent-run-plan.test.tsservices/runner/tests/unit/session-pool.test.tsservices/runner/tests/unit/wire-contract.test.tsweb/oss/src/components/AgentChatSlice/components/AgentChatEmptyState.test.tsweb/oss/src/components/AgentChatSlice/components/AgentChatEmptyState.tsxweb/oss/src/hooks/useAlwaysAllowTool.tsxweb/packages/agenta-entity-ui/package.jsonweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/PiPermissionsControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ToolManagementList.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/itemDescriptors.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/index.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/piPermissions.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/toolPermission.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/toolUtils.tsweb/packages/agenta-entity-ui/tests/unit/itemDescriptors.test.tsweb/packages/agenta-entity-ui/tests/unit/piPermissions.test.tsweb/packages/agenta-entity-ui/tests/unit/toolPermission.test.tsweb/packages/agenta-playground/tests/unit/agentRequest.test.ts
💤 Files with no reviewable changes (6)
- services/runner/src/engines/sandbox_agent/environment-setup.ts
- services/oss/tests/pytest/unit/agent/tools/test_resolution.py
- sdks/python/oss/tests/pytest/unit/agents/platform/test_resolve.py
- sdks/python/agenta/sdk/agents/handler.py
- services/runner/src/engines/sandbox_agent/session-identity.ts
- api/oss/src/core/workflows/build_kit.py
| extension on the ACP path), never over MCP. There is no forced tool set any more: the runner | ||
| activates all seven Pi built-ins on every Pi run, so `read` and `bash` are there without anything | ||
| forcing them. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align all documentation with the always-active built-in contract.
These pages retain pre-rework built-in semantics. Remove the stale claims and use the seven-name, legacy-ignored contract consistently.
- docs/design/agent-workflows/documentation/adapters/agenta.md#L34-L36: Remove the remaining forced-tool claims.
- docs/design/agent-workflows/interfaces/in-service/harness-adapters.md#L16-L23: Describe
pi_agentaas forcing skills and prompts only. - docs/design/agent-workflows/documentation/tools.md#L399-L404: Remove or qualify
web_searchbecause it is not in the seven-name runner contract. - docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md#L24-L24: Describe
builtinas legacy, accepted, and ignored.
📍 Affects 4 files
docs/design/agent-workflows/documentation/adapters/agenta.md#L34-L36(this comment)docs/design/agent-workflows/interfaces/in-service/harness-adapters.md#L16-L23docs/design/agent-workflows/documentation/tools.md#L399-L404docs/design/agent-workflows/interfaces/public-edge/agent-config-schema.md#L24-L24
There was a problem hiding this comment.
🤖 The AI agent says: Confirmed against origin/main, all four sites. adapters/agenta.md still opens with a "Forced tools: always unioned…" bullet (L8-10) that contradicts its own L34-36; harness-adapters.md still says AgentaHarness "forces the Agenta tool set" and its wire-shape table says "forced set"; tools.md L386 lists web_search among Pi's own implementations while the runner contract activates exactly the seven names; agent-config-schema.md L24 lists builtin in the runnable-type enum without the legacy note (the Default cell and the schema listing at L179 already carry the correct contract). Real follow-up: a docs sweep to align these pages with the always-active seven-name contract. Tracked as follow-up work; no code impact.
There was a problem hiding this comment.
@mmabrouk, acknowledged. The finding is valid and is out of scope for this pull request.
The follow-up should align the four documented locations with the seven always-active Pi built-ins and the legacy-ignored builtin contract. No change is required in this PR.
You are interacting with an AI system.
| `services/agent/src/protocol.ts`. A run bundles them as a `ResolvedToolSet`: the list of specs | ||
| and one `ToolCallback` (the endpoint callback tools post back to). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Normalize interface documentation paths.
The changed references do not consistently point to the current source layout.
- docs/design/agent-workflows/documentation/tools.md#L106-L107: Replace
services/agent/src/protocol.tswithservices/runner/src/protocol.ts. - docs/design/agent-workflows/interfaces/README.md#L47-L47: Make
sdk/agents/pi_builtins.pyconsistent with the documented SDK package root.
📍 Affects 2 files
docs/design/agent-workflows/documentation/tools.md#L106-L107(this comment)docs/design/agent-workflows/interfaces/README.md#L47-L47
There was a problem hiding this comment.
🤖 The AI agent says: Confirmed. services/agent/ does not exist on main; the TypeScript protocol lives at services/runner/src/protocol.ts. The same stale services/agent/... prefix also survives in several test-path references inside interfaces/README.md, so the sweep should catch those too. The sdk/agents/pi_builtins.py vs full sdks/python/agenta/sdk/agents/... root is the same normalization. Folding into the docs-sweep follow-up.
There was a problem hiding this comment.
@mmabrouk, acknowledged. The docs sweep should replace the stale services/agent/... references and normalize the SDK paths to sdks/python/agenta/sdk/.... The current finding remains open until that follow-up applies the corrections.
You are interacting with an AI system.
| 2. Permission control for Pi happens only through the main policy | ||
| (`runner.permissions.default`: `allow`, `allow_reads`, `ask`, `deny`) and the three editable | ||
| rule lists `harness.permissions.{allow, ask, deny}`. The default is unchanged: all three lists | ||
| empty, policy `allow_reads`, so `read`, `grep`, `find` and `ls` run unattended and `bash`, | ||
| `edit` and `write` show the approval card. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Scope the “only” claim to built-in permission control.
The shared tool model still allows an explicit permission on custom tools to override authored rules and the default. If this item describes only the seven Pi built-ins, say so explicitly; otherwise “permission control for Pi happens only through” is too broad.
There was a problem hiding this comment.
🤖 The AI agent says: Fair wording nit, verified: a custom tool's explicit permission does override the default at the runner (permission-plan.ts carries the resolved spec's explicit author permission into effectivePermission). The "only" holds for the seven built-ins — which can no longer carry per-tool config anywhere — but not for Pi runs as a whole. The item will be scoped to the built-ins explicitly in the docs sweep.
There was a problem hiding this comment.
@mmabrouk, agreed. The statement should apply only to the seven Pi built-ins. Custom tools can still use an explicit permission override. I will keep the comment open until the documentation update is complete.
✏️ Learnings added
Learnt from: mmabrouk
Repo: Agenta-AI/agenta PR: 5651
File: docs/design/agent-workflows/projects/default-agent-builtins/addendum-always-active.md:33-37
Timestamp: 2026-08-02T17:48:33.669Z
Learning: In `services/runner/src/permission-plan.ts`, a resolved custom tool specification can provide an explicit `permission` that overrides authored harness permission rules and the default permission. This override does not apply to the seven Pi built-ins because they cannot carry per-tool configuration.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
| builtin_names=builtin_names, | ||
| tool_specs=tool_specs, | ||
| ) | ||
| _validate_unique_names(tool_specs) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Validate reserved names before resolving custom tools.
Line 229 runs after secret lookup and adapter resolution. A CodeToolConfig named read with a missing secret raises MissingToolSecretError instead of ReservedToolNameError. A reserved gateway or reference name can also reach its adapter before rejection.
Validate declared custom names before secret or adapter work. Keep the final validation for names produced by adapters.
Based on PR objectives, custom tool names must not use reserved built-in names.
There was a problem hiding this comment.
🤖 The AI agent says: Confirmed on the facts: _validate_unique_names runs last (L229), so a reserved-named CodeToolConfig with a missing secret surfaces MissingToolSecretError instead of ReservedToolNameError, and reserved-named gateway/reference/platform configs reach their resolver adapters before rejection. Two qualifiers on severity: the reserved name is still always rejected before any spec leaves resolve(), so nothing ever reaches the runner — this is error-precedence plus wasted adapter calls, not a gate bypass; Minor rather than Major. The right fix matches the suggestion: validate declared config names up front and keep the final pass for adapter-produced names. Real follow-up, tracked.
There was a problem hiding this comment.
@mmabrouk, agreed. This is a Minor issue.
The current code does not allow a reserved custom tool to reach the runner. The impact is incorrect error precedence and unnecessary resolver work.
The follow-up should validate declared custom tool names before secret lookup and adapter resolution. The final _validate_unique_names(tool_specs) call should remain to validate adapter-produced tool specifications.
You are interacting with an AI system.
| async def test_a_bare_tool_name_string_is_ignored_too(): | ||
| # `coerce_tool_config` turns a bare string into a BuiltinToolConfig. | ||
| resolved = await ToolResolver().resolve(coerce_tool_configs(["read"])) | ||
|
|
||
| assert resolved.tool_specs == [] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Pass parsed tool configurations to ToolResolver.resolve.
coerce_tool_configs() returns ToolConfigParseResult, not Sequence[ToolConfig]. This test does not reliably exercise the coerced BuiltinToolConfig. Pass coerce_tool_configs(["read"]).tool_configs to the resolver.
Proposed fix
- resolved = await ToolResolver().resolve(coerce_tool_configs(["read"]))
+ parsed = coerce_tool_configs(["read"])
+ resolved = await ToolResolver().resolve(parsed.tool_configs)Based on PR objectives, legacy builtin entries must be ignored by the resolver.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def test_a_bare_tool_name_string_is_ignored_too(): | |
| # `coerce_tool_config` turns a bare string into a BuiltinToolConfig. | |
| resolved = await ToolResolver().resolve(coerce_tool_configs(["read"])) | |
| assert resolved.tool_specs == [] | |
| async def test_a_bare_tool_name_string_is_ignored_too(): | |
| # `coerce_tool_config` turns a bare string into a BuiltinToolConfig. | |
| parsed = coerce_tool_configs(["read"]) | |
| resolved = await ToolResolver().resolve(parsed.tool_configs) | |
| assert resolved.tool_specs == [] |
There was a problem hiding this comment.
🤖 The AI agent says: Confirmed — the test is vacuous. coerce_tool_configs returns a ToolConfigParseResult (a pydantic model), and iterating a pydantic model yields (field, value) tuples, so resolve() sees no BuiltinToolConfig at all and returns an empty set for the wrong reason: green without exercising the coercion path. The fix is exactly coerce_tool_configs(["read"]).tool_configs. Real follow-up, tracked.
There was a problem hiding this comment.
@mmabrouk, confirmed. The test must pass coerce_tool_configs(["read"]).tool_configs to ToolResolver.resolve().
The current test does not validate the coercion-to-resolution path. I will leave the finding open until the follow-up patch is available.
You are interacting with an AI system.
| () => (Array.isArray(config.tools) ? (config.tools as unknown[]) : null), | ||
| [config.tools], | ||
| ) | ||
| const hasPiPermissions = isPiHarness |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Treat an omitted harness kind as pi_core.
isPiHarness is false when harness.kind is absent. The runner resolves an absent harness to pi_core, so this UI hides the Pi permission rules while Pi built-ins remain active. Include the absent value in the Pi condition and add coverage for {harness: {}}.
Proposed fix
- const isPiHarness = harnessValue === "pi_core" || harnessValue === "pi_agenta"
+ const isPiHarness =
+ !harnessValue || harnessValue === "pi_core" || harnessValue === "pi_agenta"Based on supplied runner contract, services/runner/src/engines/sandbox_agent/pi-model-config.ts resolves a missing harness as pi_core.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasPiPermissions = isPiHarness | |
| const isPiHarness = | |
| !harnessValue || harnessValue === "pi_core" || harnessValue === "pi_agenta" |
There was a problem hiding this comment.
🤖 The AI agent says: Confirmed real. useModelHarness.tsx leaves harnessValue null when harness.kind is absent, so isPiHarness is false and the Pi permission rules UI is hidden — while the runner resolves a missing harness to pi_core (pi-model-config.ts isPiHarness, mirroring buildRunPlan) with all seven built-ins active and gated. Any config authored without harness.kind (SDK- or API-side) hits this. The one-line fix (!harnessValue || …) plus a {harness: {}} test case is right. One severity note: the built-ins still run under the default allow_reads policy, so this hides the controls rather than dropping the gate — a UI visibility gap, not a permission bypass. Real follow-up, tracked.
There was a problem hiding this comment.
@mmabrouk Confirmed. The severity is a UI visibility gap, not a permission bypass.
When harness.kind is absent, the runner uses pi_core and applies the permission gate. The UI must use the same default so it renders PiPermissionsControl. The tracked fix should include the omitted-kind case.
🐇
✏️ Learnings added
Learnt from: mmabrouk
Repo: Agenta-AI/agenta PR: 5651
File: web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx:379-379
Timestamp: 2026-08-02T17:49:02.474Z
Learning: For Pi agent configurations, the runner resolves an omitted `harness.kind` as `pi_core`. In `web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx`, Pi-specific permission controls must therefore treat an absent harness kind as Pi. Without this handling, the UI hides `PiPermissionsControl` while the runner still activates Pi built-ins and gates them with the default `allow_reads` permission policy. This is a UI visibility gap, not a permission bypass.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Railway Preview Environment
|
Context
Every new Pi agent showed four rows in its Tools list: Read, Bash, Edit and Write. They looked like
tools you could add, remove or configure, but they were none of those things. Next to them the
Permissions area showed a read-only "Auto-approve" panel that listed rules you could not edit, and a
separate built-in multi-select that wrote back into the same
toolsarray. Three surfaces, oneunderlying setting, no clear owner.
Those four rows exist because of issue #5590. A Pi agent started from a trigger arrived at the
runner with
tools: []. The runner read that field as the built-in grant list, granted nothing, andthe agent ran with no tools at all, so it could not read the mount it was created for. The
playground never showed the bug because the build kit injects tools at run time. The v0.107 fix put
four explicit
{"type": "builtin", "name": ...}entries in the default template, which fixed thetrigger case but placed Pi-specific machinery in the top-level
tools[]array and produced theconfusing UI above.
This PR removes built-ins from the config entirely. That makes the #5590 class impossible (there is
no grant list to be empty) and fixes the presentation at the same time.
What this changes
All seven Pi built-ins (read, bash, edit, write, grep, find, ls) are now active on every Pi run.
Our runner activates all seven each time it starts Pi; Pi on its own activates only four. Activation
is unconditional platform behavior, so the built-ins appear nowhere in the agent config and nowhere
in the Tools list.
Permission control for Pi now happens in exactly two places: the main policy
(
runner.permissions.default:allow|allow_reads|ask|deny) and the three editable rulelists
harness.permissions.{allow, ask, deny}. Defaults are unchanged: all three lists empty, policyallow_reads, so read/grep/find/ls run unattended while bash/edit/write show the approval card.The Permissions area for a Pi agent now shows those three lists as tag selects, seeded with the seven
canonical names (Read, Bash, Edit, Write, Grep, Find, Ls) and accepting free-typed pattern rules such
as
Bash(npm run:*). The view-only Auto-approve panel and the built-in multi-select are gone. A rulecreated by the "Always allow" switch on an approval card lands in that same Allow list and can be
removed there.
Rule matching for the seven built-ins is now case-insensitive, so
bash,BashandBASHall namethe same tool. Custom tool names stay case-significant, and the body of a prefix pattern stays
case-sensitive because a shell command's case matters. Unknown tool names at the gate still fail
closed.
Old configs keep working. A
{"type": "builtin", "name": ...}entry intools[]is accepted,ignored, and logged once per entry. It no longer occupies the custom-tool name namespace and renders
nowhere in the UI. Nothing writes new ones: the SDK template builder, the service's
agent.jsonandDEFAULT_TOOLS, the static catalog entry that seeds new accounts, and the build-kit authoringinstructions all stop emitting them.
The
AGENTA_FORCED_TOOLSmachinery in the Agenta harness (a forced read+bash union) is obsolete nowthat all seven are always on, and is deleted along with
force_toolsand the API build-kit spreadthat carried it.
Before / after
The default template's tools array:
How you allow bash without an approval card:
The runner's plan no longer depends on the request's
toolsfield:Decisions worth reviewing, and what they cost
A denied tool is refused at call time, not hidden from the model. The seven built-ins are always
advertised, so a
deny: ["Bash"]rule does not remove bash from the model's tool list. The model canstill call it; the call is attempted and terminates as
tool-output-deniedwith"Denied by the permission policy.". The cost is one wasted model turn and prose that may mentionthe attempt. The gain is a stable prompt (the same tool list regardless of rules, which keeps the
prompt cache warm) and a visible audit trail of what the agent tried. Live QA confirmed the wire
outcome (QA-4, QA-9).
Policy
allowwidens for agents that have custom tools. Under the old grant-list model, an agentwith custom tools and policy
allowended up with no built-ins at all, which was the #5590 bug. Suchan agent now runs all seven, and under
allowthat means bash, edit and write run unattended with nocard. This is the intended fix, but it is a real behavior change for any existing agent in that
state, so it belongs in the release notes.
grep, find and ls become visible to every Pi agent. They are classified read-only, so under the
default
allow_readspolicy they auto-run without a card, exactly like read. Agents that previouslyhad only four built-ins gain three read tools they did not have. The read-only classification is
pinned in the shared golden fixture so it cannot be flipped by accident.
The deprecated
toolswire field keeps being sent, filled with all seven names. The new runnerignores the field. An old runner (a self-hosted deployment whose API updates before the runner image)
still reads it as the grant list, and sending seven names gives it the same tool set the new runner
activates. Sending
[]there would recreate #5590 during a version skew; omitting the key would givePi's own four. Claude still sends
[], unchanged.Built-in names are now reserved for custom tools. A custom tool named
readused to shadow thebuilt-in silently. The resolver now raises
ReservedToolNameError, and the runner skips a collidingspec as a second line of defense. The reservation is harness-agnostic (the resolver has no harness
context), so a Claude config that names a custom tool
writeis refused too. That is stricter thanstrictly necessary for Claude and is the one place this change can break a config that used to load.
No data migration. Revisions saved since 107 keep their four legacy entries in the database. A
silent rewrite on read raises a false "unsaved changes" marker the moment a drawer opens, and a
rewrite on commit mutates author data for no runtime benefit. The entries are inert. The
builtinarm of the config union should be removed one release from now, together with a permissive fallback
for unknown
typevalues so older revisions still load.New and changed user-visible strings
Everything below is either new or reworded. Nothing else user-facing changed.
In the Permissions editor (
PiPermissionsControl):Allow,Ask,Deny.that are never allowed to run."
Bash(npm run:*).In the config schema tooltips (
_HarnessPermissionsSchemaand_HarnessSchemainsdks/python/agenta/sdk/utils/types.py):allow,askanddenyfields.permissions: "The allow / ask / deny rules that decide which tools may run." (was Claude-onlywording).
mode: "Claude's default permission mode (default / acceptEdits / plan / bypassPermissions). Notused by Pi."
In run errors:
tool."
Log lines only (not shown in the product):
are no longer configured here; this entry was ignored (use harness.permissions to allow, ask or
deny it)".
One behavior detail with no new string: a rule created by the card's "Always allow" switch now reads
Bashinstead ofbash, matching the spelling the dropdown offers.Tests
86 files changed. Every suite below was re-run after the final change.
pnpm run typecheckthenpnpm testpytest oss/tests/pytest/unit -qpytest oss/tests/pytest/unit -qpytest oss/tests/pytest/unit -qpnpm --filter @agenta/oss exec tsgo --noEmit@agenta/entity-ui/@agenta/playground/@agenta/ossunitpnpm lint-fixinweb;ruff format+ruff checkWhat the new tests pin, beyond the rewrites that follow the deleted code:
golden/pi_builtin_tools.json) now holds the canonical table of seven names,their rule-name capitalization and their read-only flag. Python asserts the names, the runner
asserts the whole table, and the runner extension's own copy of the list is asserted against it
too, so all three copies drift-check against one file.
toolsfield in any form, and gating follows thepolicy alone.
case-significant custom tool name, and deny outranking ask outranking allow across mixed case.
the same pattern from ask and deny first), and the tool-count filters that keep the Tools header,
the list body and the conversation card in agreement on a legacy config.
The agent release gate resources are updated in the same change: the probe and product templates no
longer seed Pi tool lists, and four new journeys cover the permission model end to end (
rule_deny,rule_allow,rule_case,builtin_grep), all skipped on a non-Pi cell and documented in the gate'scoverage.md.Live QA
Run twice against the EE dev stack serving this tree, with the runner container restarted first
(runner TypeScript does not hot-reload). Round 1 found one blocker and one cosmetic issue; both were
fixed and round 2 re-ran all nine items end to end. Round 2: nine of nine pass, no failures.
"tools": []; the static catalog seed carries noentries either.
names, and keeps
Bash(npm run:*)as one tag.ls,grepandfindall executed with real output and no approval card.deny: ["Bash"]produced an attempted call that terminated astool-output-denied, no card, noexecution, run ended normally. Uppercase
BASHin deny behaved identically.allow: ["Bash"]and lowercaseallow: ["bash"]both ran bash unattended.Bashinto the editor's own Allow list, the re-run needed nocard, and removing the tag brought the card back.
anywhere, shows the same three empty rule lists, runs grep normally, and logs one ignored-entry
warning per dropped entry.
playground (no build-kit injection), executed both
lsandgrepand still listed its custom tool.Round 1's blocker was a "4 tools" chip on the playground conversation card, which counted the
ignored legacy entries; it now uses the same filter as the other three surfaces.
The stack has no reachable trigger target, so the non-playground path was exercised by calling
POST /services/agent/v0/invokedirectly with the committed revision's parameters. That is the samepath a trigger takes minus the scheduler.
What reviewers should double-check
ReservedToolNameErrorfires for any harness, so aClaude agent with a custom tool named
read,bash,edit,write,grep,findorlsnowfails to resolve where it used to work. If we have such configs in the wild, this needs to be
scoped to Pi.
allowplus custom tools. Confirm we are comfortable that agents in this state go fromno built-ins to seven unattended built-ins, including bash, edit and write.
toolswire field carrying seven names. An old runner receiving them computesbuiltinGatingActive = trueeven under policyallow, so built-in calls route through theextension dialog, which resolves to allow. Behavior is the same; only the code path differs.
config-writing agent stops producing legacy entries. If any other generator writes
tools, itneeds the same treatment before the
builtinarm is removed.j_rule_denyis still assertion-loose. The journey asserts the bash outcome is present and not"available", and reports the observed value. Live QA pinned that value atdeniedin bothrounds, so the assertion can now be tightened to
== "denied". It has not been tightened in thisPR; say the word and it is a one-line change.
What to QA
built-in rows, and the committed config has
"tools": [].the seven names, and
Bash(npm run:*)typed into Allow stays a single tag.grepsomething and tolsits working directory. Both run with no approval card.Bashin Deny, ask it to run a shell command. The call is attempted and refused, and no outputcomes back. Swap Deny for Allow and the same command runs with no card.
shows up as
Bashin the Allow list, the next run needs no card, and deleting the tag brings thecard back.
entries. No tool rows, no tool count on the conversation card, and it still runs normally.
Follow-ups
builtinarm from the tool-config union one release from now, with a permissivefallback for unknown
typevalues so older revisions still load.j_rule_denyto assertbash_outcome == "denied"now that live QA has pinned the value.https://claude.ai/code/session_01McMogkcDRV7UpSAjfd8VKG