refactor(agent): collapse run-selection into AgentConfig, rename harness_kwargs#4840
Conversation
dosubot
Bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
refactoring
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR updates agent-workflow docs, SDK models, service wiring, and web request builders so run-selection fields live on ChangesAgent Config Structure Cleanup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
Changes made — maps the approved #4821 review comments to the diff. #4821 c3470018175 + c3470081368 ("harness and sandbox are part of AgentConfig — don't have anything outside
#4821 c3469634113 (general
Wire impact: none. The Tests: SDK agents 342, service-agent 38, FE playground 121 (agentRequest 19, +1 new nested-under-agent test), entity-ui 126. ruff + prettier + eslint clean. Codex (xhigh) reviewed (runtime sound, wire unchanged; reworded a now-inaccurate Docs: synced the inventory pages (agent-config-schema, agent-messages, workflow-invoke, agent-service-handler, neutral-runtime-dtos) + documentation pages (agent-configuration, ground-truth, ports-and-adapters, architecture, harness-adapters, adapters/pi, adapters/agenta, agent-template) to the new shape. Deferred: GitButler note: hit the documented stale- |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Railway Preview Environment
Updated at 2026-06-25T12:05:32.499Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 615bf410-9144-4148-9e01-f5971b298a17
📒 Files selected for processing (33)
docs/design/agent-workflows/documentation/adapters/agenta.mddocs/design/agent-workflows/documentation/adapters/pi.mddocs/design/agent-workflows/documentation/agent-configuration.mddocs/design/agent-workflows/documentation/agent-template.mddocs/design/agent-workflows/documentation/architecture.mddocs/design/agent-workflows/documentation/ground-truth.mddocs/design/agent-workflows/documentation/ports-and-adapters.mddocs/design/agent-workflows/interfaces/in-service/agent-service-handler.mddocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/agent-workflows/interfaces/in-service/neutral-runtime-dtos.mddocs/design/agent-workflows/interfaces/public-edge/agent-config-schema.mddocs/design/agent-workflows/interfaces/public-edge/agent-messages.mddocs/design/agent-workflows/interfaces/public-edge/workflow-invoke.mddocs/design/agent-workflows/projects/agent-config-structure-cleanup/status.mdsdks/python/agenta/__init__.pysdks/python/agenta/sdk/agents/__init__.pysdks/python/agenta/sdk/agents/adapters/claude_settings.pysdks/python/agenta/sdk/agents/adapters/harnesses.pysdks/python/agenta/sdk/agents/dtos.pysdks/python/agenta/sdk/agents/utils/wire.pysdks/python/agenta/sdk/utils/types.pysdks/python/oss/tests/pytest/unit/agents/adapters/test_claude_settings.pysdks/python/oss/tests/pytest/unit/agents/test_dtos_agent_config.pysdks/python/oss/tests/pytest/unit/agents/test_harness_adapters.pysdks/python/oss/tests/pytest/unit/agents/test_wire_contract.pyservices/oss/src/agent/app.pyservices/oss/tests/pytest/unit/agent/test_default_agent_config.pyservices/oss/tests/pytest/unit/agent/test_select_backend.pyweb/oss/src/components/AgentChatSlice/assets/transport.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentConfigControl.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/ClaudePermissionsControl.tsxweb/packages/agenta-playground/src/state/execution/agentRequest.tsweb/packages/agenta-playground/tests/unit/agentRequest.test.ts
💤 Files with no reviewable changes (2)
- sdks/python/agenta/init.py
- sdks/python/agenta/sdk/agents/init.py
| | harness | yes, enum | yes, on `AgentConfig` | wired, picks the harness class | Enum-enforced. The runtime validates via `make_harness`. | | ||
| | sandbox | yes, enum | yes, on `AgentConfig` | wired to the backend, absent from `SessionConfig` | Backend concern, not agent identity. | | ||
| | permission_policy | yes, enum | yes, on `AgentConfig` | wired to `SessionConfig` | Only the Claude harness reads it. Pi ignores it, so it is decorative for `pi_core` and `pi_agenta`. | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reword the permission_policy row to match the new ownership.
This still says the policy is wired to SessionConfig, but the flattened contract keeps it on AgentConfig and the runtime consumes it through harness wiring. Please update the wording so ownership stays on the agent config. Per the PR objective, permissionPolicy still rides through wire_tools().
|
Addressed CodeRabbit's two substantive findings (commit
Skipped the Tests green: SDK dtos 21, playground agentRequest 20; ruff/prettier clean; typecheck clean. |
mmabrouk
force-pushed
the
feat/agent-model-picker
branch
from
0e7f4f7 to
e282ea4
Compare
mmabrouk
force-pushed
the
feat/agent-config-structure-cleanup
branch
from
8aa3c90 to
372b7a8
Compare
mmabrouk
force-pushed
the
feat/agent-model-picker
branch
from
e282ea4 to
efd5f70
Compare
mmabrouk
force-pushed
the
feat/agent-config-structure-cleanup
branch
from
372b7a8 to
2610078
Compare
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Looks good to me. Please merge
mmabrouk
added
lgtm
mmabrouk
force-pushed
the
feat/agent-config-structure-cleanup
branch
from
2610078 to
cec8c43
Compare
…ess_kwargs Move harness/sandbox/permission_policy from the separate RunSelection DTO into AgentConfig (one agent definition, under data.parameters.agent). Retire RunSelection. Rename the per-harness options bag harness_options -> harness_kwargs. The /run wire is unchanged (golden fixtures untouched). PR #4821 review comments 2/7/8. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
…lection keys on FE
|
You already lgtm'd this; the only change since is a small CodeRabbit fix (an explicit empty |
mmabrouk
force-pushed
the
feat/agent-model-picker
branch
from
673b178 to
c406a5d
Compare
mmabrouk
force-pushed
the
feat/agent-config-structure-cleanup
branch
from
cec8c43 to
3f051c8
Compare
The inspect-picker port pulled two names from the stacked feat/agent-config-structure-cleanup (#4840), whose backend is not in big-agents yet: the harness_options -> harness_kwargs rename and a pi_core default harness. big-agents' backend still reads the harness_options key and only knows pi/agenta/claude, so on big-agents the renamed bag would be dropped and a harness-less config would send an unknown harness. Revert both to match the big-agents contract: write the Claude permission bag under harness_options, and default an unset harness to "pi". The nested parameters.agent shape is kept (big-agents' RunSelection.from_params already reads it), and the inspect-driven picker is kept (it falls back to the schema catalog when /inspect publishes no models). Flipping these back is a one-line follow-up once #4840 lands on big-agents. tsc 0, eslint clean, playground unit tests pass (28).
…ents Rebuilds the contained agent config-panel branch on top of big-agents after the agent stack landed (#4830 canonical /inspect + models, #4839 model picker, #4840 collapse run-selection into AgentConfig + harness_kwargs). The inspect-driven picker, connectionUtils, inspectMeta and EnumSelectControl we had ported now live in big-agents verbatim, so they collapse out — this branch is only our genuine UX delta on top: - AgentConfigControl as schema-driven accordion sections (ConfigAccordionSection primitive) - per-item drawers on the shared EnhancedDrawer (ConfigItemDrawer) with lazy content - two-pane skill editor, MCP server form, tool form, harness select, markdown/code/JSON editors - Authentication as radio cards with explainers; live JSON-edit sync - chat panel: inline per-turn run errors + truncation, empty-turn collapse - playground wiring (header menu view modes, height calc, router) Coherent with the merged backend: harness/sandbox/permission_policy ride parameters.agent, harness_kwargs bag, pi_core default — taken from big-agents (the earlier Option-A reverts are undone). tsc 0 (entity-ui/agenta-ui/entities/playground; oss 593 baseline, 0 new), eslint clean, unit tests pass (entity-ui 126, playground 122, entities 683).
1 participant
What and why
PR #4821 review (comments 2 / 7 / 8) asked for one thing: there should be a single agent config, not an
AgentConfigplus a separate sidecarRunSelection. The run-selection fields were split off into their own DTO and parsed separately, so "what the agent is" and "how it runs" lived in two places that both rodedata.parameters.This collapses them.
harness,sandbox, andpermission_policyare now plain fields onAgentConfig, parsed by the oneAgentConfig.from_params(...).RunSelectionis retired. The per-harness options bagharness_optionsis renamed toharness_kwargs(comment 8): a map keyed by harness name where the author sets per-harness knobs, e.g. Pi'ssystem/append_systemunderpi_core, Claude'spermissionsunderclaude.permission_policystays the sidecar action-permission, now insideAgentConfig.POC: no back-compat, no deprecation shims. Shapes change cleanly.
Public-edge envelope change
The run-selection fields move from siblings of
agentto inside it. They live underdata.parameters.agent.{harness,sandbox,permission_policy}, notdata.parameters.{harness,sandbox,permission_policy}.Before:
After:
The playground schema (
AgentConfigSchema) andbuild_agent_v0_defaultalready placed these insideagent, so the schema and default did not change. The FE request builders (agentRequest.ts,AgentChatSlice/transport.ts) stopped injecting top-levelharness/sandboxand now default them inside theagentblock.The
/runwire did NOT changeharness/sandboxride the wire as explicit args torequest_to_wire(harness=, sandbox=)(the harness adapter passes them frommake_harness(agent_config.harness, ...)+select_backend(agent_config).sandbox);permissionPolicyrideswire_tools(). There are no new wire fields and the golden fixtures are untouched. This is an envelope/config-placement + parsing change, not a contract change.sandboxstays a backend/environment concern read before the session is built; no adapter consumes it.Decisions
RunSelection. Nothing else needed it; one agent definition is the whole point of the comment. Removed its export fromagenta.sdk.agentsand the top-levelagentapackage.harness_options->harness_kwargs. Today's bag already was the general per-harness map the comment described, so this is a reconcile-rename, not a new field.sandboxleft in place (not removed). The sibling sidecar-uri-config work will replacesandboxwith an optionalurinext; this PR movessandboxcleanly intoAgentConfigso that follow-up is a small field swap.Tests
agent). entity-ui: 126.Docs
Updated the interface inventory pages (agent-config-schema, agent-messages, workflow-invoke, agent-service-handler, neutral-runtime-dtos) and the affected documentation pages (agent-configuration, ground-truth, ports-and-adapters, architecture, harness-adapters, adapters/pi, adapters/agenta, agent-template) to the "everything under AgentConfig" shape and the
harness_kwargsname.Deferred
services/agent/src/protocol.ts:354has aharness_optionsdoc comment (describes the Python-side bag). It is in the runner dir (sibling HTTP-MCP impl active there), so untouched; rename in a runner-owned change.https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc