feat(agent): playground build kit (default agent config)#4929
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughIntroduces a server-generated "Playground Build Kit" agent template overlay returned in ChangesPlayground Build Kit Overlay
Sequence Diagram(s)sequenceDiagram
participant Client
participant SimpleApplicationsRouter
participant build_agent_template_overlay
participant Frontend State
participant buildAgentRequest
Client->>SimpleApplicationsRouter: GET /simple-application
SimpleApplicationsRouter->>build_agent_template_overlay: call
build_agent_template_overlay-->>SimpleApplicationsRouter: overlay dict
SimpleApplicationsRouter-->>Client: SimpleApplicationResponse{additional_context: {playground_build_kit: {agent_template_overlay}}}
Client->>Frontend State: store inspect response
Frontend State->>Frontend State: workflowAgentTemplateOverlayAtomFamily derives overlay
Client->>buildAgentRequest: invoke
buildAgentRequest->>Frontend State: read workflowBuildKitEnabledAtomFamily
buildAgentRequest->>Frontend State: read workflowAgentTemplateOverlayAtomFamily
buildAgentRequest->>buildAgentRequest: applyBuildKitOverlay(config, overlay)
buildAgentRequest-->>Client: merged parameters for /invoke
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
🤖 The AI agent says: @Agenta-AI please review — DRAFT, do not merge. Specific feedback wanted:
Implemented by Codex (gpt-5.5, xhigh). Change 1 (collapsible drawer sections) intentionally excluded. |
|
🤖 The AI agent says: What to review on this PR:
|
| agent_template_overlay: Optional[dict] = Field( | ||
| default=None, | ||
| description="Partial `parameters.agent` overlay applied by the playground only.", | ||
| ) |
There was a problem hiding this comment.
🤖 The AI agent says: 📄 Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Model agent_template_overlay explicitly instead of using dict.
This is now a backend/frontend contract, but Optional[dict] leaves the OpenAPI schema and runtime validation opaque. Please promote the overlay shape into concrete Pydantic models (or typed submodels for tools, skills, and sandbox) so downstream clients get a stable contract. As per coding guidelines, "Define explicit request and response models in models.py."
There was a problem hiding this comment.
My guess here is that this is a subset of parameters.agent. not sure how we can specify it, if possible why not, obviously in a way that uses the paramters.agent so that we dont need to change both each time
There was a problem hiding this comment.
🤖 The AI agent says: Modeled it explicitly as AgentTemplateOverlay (Pydantic) with tools/skills/sandbox, so the inspect contract is no longer an opaque dict. On reusing parameters.agent directly: there is no shared Pydantic model for the authoring template today — it rides as free-form data.parameters — and the SDK runtime AgentTemplate is the flattened parse with different field names, so neither can be reused 1:1 to type this overlay. The new model documents that it is a deliberate subset of that shape. Done in d07809e.
| revision = catalog.retrieve_revision(slug=slug) | ||
| if revision and revision.flags and revision.flags.is_skill: | ||
| continue | ||
| slugs.append(slug) |
There was a problem hiding this comment.
🤖 The AI agent says: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Tighten the reserved-workflow filter.
This currently appends reserved slugs even when retrieve_revision() returns None or revision.flags is missing. The expected overlay only includes confirmed non-skill static workflows, so this can leak invalid tool embeds into the playground.
revision = catalog.retrieve_revision(slug=slug)
- if revision and revision.flags and revision.flags.is_skill:
+ if not revision or not revision.flags or revision.flags.is_skill:
continue
slugs.append(slug)
| ), | ||
| ) | ||
| if simple_application | ||
| else None, |
There was a problem hiding this comment.
🤖 The AI agent says: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don't let build-kit synthesis blank the whole fetch response.
fetch_simple_application() is wrapped in @suppress_exceptions(default=SimpleApplicationResponse()). If build_agent_template_overlay() raises, this path now returns an empty 200 response instead of the fetched application. Build the overlay behind a local try/except and fall back to additional_context=None so the inspect path still works. As per path instructions, use @suppress_exceptions(...) only for controlled defaults.
| if (!(key in userPermissions)) return false | ||
| return stableString(userPermissions[key]) !== stableString(overlayValue) | ||
| }) | ||
| .map(([key]) => key) |
There was a problem hiding this comment.
🤖 The AI agent says: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include overlay-added permissions in the override list.
The key in userPermissions guard drops permissions that the build kit adds on top of the draft. In the common write_files/new-network-key case, the warning next to SandboxPermissionControl under-reports the effective playground permissions.
function overriddenPermissionKeys(
userPermissions: Record<string, unknown> | null | undefined,
overlayPermissions: Record<string, unknown> | null | undefined,
): string[] {
- if (!userPermissions || !overlayPermissions) return []
+ if (!overlayPermissions) return []
return Object.entries(overlayPermissions)
- .filter(([key, overlayValue]) => {
- if (!(key in userPermissions)) return false
- return stableString(userPermissions[key]) !== stableString(overlayValue)
- })
+ .filter(
+ ([key, overlayValue]) =>
+ stableString(userPermissions?.[key]) !== stableString(overlayValue),
+ )
.map(([key]) => key)
}
| const [buildKitEnabled, setBuildKitEnabled] = useAtom( | ||
| useMemo(() => workflowBuildKitEnabledAtomFamily(revisionId ?? ""), [revisionId]), | ||
| ) | ||
| const [buildKitExpanded, setBuildKitExpanded] = useState(true) |
There was a problem hiding this comment.
🤖 The AI agent says: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cancel won't restore the build-kit toggle.
buildKitEnabled now lives outside the config snapshot, but cancelSection() only rolls back value. In drawer layouts, toggling this switch and pressing Cancel still changes later playground runs.
+ const buildKitSnapshot = useRef<boolean | null>(null)
+
const openSectionDrawer = useCallback(
(key: "model-harness" | "advanced") => {
sectionSnapshot.current = value ?? {}
+ buildKitSnapshot.current = key === "advanced" ? buildKitEnabled : null
setOpenSection(key)
},
- [value],
+ [value, buildKitEnabled],
)
const cancelSection = useCallback(() => {
if (sectionSnapshot.current) onChange(sectionSnapshot.current)
+ if (openSection === "advanced" && buildKitSnapshot.current != null) {
+ setBuildKitEnabled(buildKitSnapshot.current)
+ }
setOpenSection(null)
- }, [onChange])
+ }, [onChange, openSection, setBuildKitEnabled])
| buildKitExpanded && "rotate-90", | ||
| )} | ||
| /> | ||
| </button> |
There was a problem hiding this comment.
🤖 The AI agent says: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Move the switch out of the header button.
Switch is an interactive control, so nesting it inside the clickable <button> creates invalid interactive content and can break keyboard/focus handling. Keep the header toggle and the switch as separate controls.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/packages/agenta-playground/tests/unit/agentRequest.test.ts (1)
285-305: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the actual commit serialization path here.
This only proves
buildAgentRequest()leaves the configuration atom untouched. It would still pass if the commit serializer later started merging the overlay, which is the regression this test name claims to guard against. A commit-path assertion should read through the same selector/helper thatweb/packages/agenta-entities/src/workflow/state/commit.tsuses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d66d4d68-f4bd-4025-b2f9-7adffbb914f2
📒 Files selected for processing (16)
api/oss/src/apis/fastapi/applications/models.pyapi/oss/src/apis/fastapi/applications/overlay.pyapi/oss/src/apis/fastapi/applications/router.pyapi/oss/tests/pytest/unit/applications/test_build_kit_overlay.pyservices/oss/src/agent/schemas.pyservices/oss/tests/pytest/unit/agent/test_default_agent_template.pyweb/packages/agenta-entities/src/workflow/api/api.tsweb/packages/agenta-entities/src/workflow/index.tsweb/packages/agenta-entities/src/workflow/state/commit.tsweb/packages/agenta-entities/src/workflow/state/index.tsweb/packages/agenta-entities/src/workflow/state/store.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsxweb/packages/agenta-playground/src/state/execution/agentRequest.tsweb/packages/agenta-playground/src/state/execution/index.tsweb/packages/agenta-playground/src/state/index.tsweb/packages/agenta-playground/tests/unit/agentRequest.test.ts
mmabrouk
force-pushed
the
feat/client-tool-roundtrip-4920
branch
from
be5bd45 to
76b89f4
Compare
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
a2d4cd1 to
1a8f483
Compare
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
c1d6561 to
332c5d8
Compare
There was a problem hiding this comment.
The code here looks fine. I have a feeling we are putting everything into this agent request file, and some of these feel illogical about overlays and helpers. If that's the way we usually do it on the front end, I am all right with it, but if usually there is a better organization and structure in cases like this one, then let's use that. Check that out and see if you want to improve the structure, work with it, the functions, etc., or keep it as is
There was a problem hiding this comment.
🤖 The AI agent says: Agreed — extracted the overlay logic out of agentRequest.ts into a dedicated web/packages/agenta-playground/src/state/execution/buildKitOverlay.ts (the applyBuildKitOverlay / withBuildKitOverlay merge helpers and their identity/deep-merge utilities). agentRequest.ts now just imports and applies it, so the request builder stays focused on composing the /invoke envelope. applyBuildKitOverlay is still re-exported from agentRequest.ts so existing consumers keep their import path. Done in d07809e.
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
d07809e to
65f298a
Compare
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
65f298a to
b4873b7
Compare
mmabrouk
force-pushed
the
feat/client-tool-roundtrip-4920
branch
from
af0e4d9 to
d366270
Compare
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
b4873b7 to
ee0a6c0
Compare
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
ee0a6c0 to
86b1d55
Compare
mmabrouk
changed the base branch from
feat/client-tool-roundtrip-4920
to
big-agents
1 participant
Context
When a user opens the playground to build a new agent, the assistant needs authoring scaffolding: the platform ops (find capabilities, commit a revision, etc.), the Agenta getting-started skill, and elevated sandbox permissions so it can write and execute files. That scaffolding is a build aid, not the user's app. The committed agent must carry only what the user authored.
Previously
build_agent_v0_default()baked these into the published default, so every deployed agent shipped with platform tools and sandbox elevation baked in. This PR fixes that by separating "what the playground injects for a build run" from "what gets committed."Design:
docs/design/agent-workflows/projects/default-agent-config/design.mdChanges
The backend now builds a read-only agent-template overlay and attaches it to the inspect response. The frontend merges it onto
parameters.agentonly for a playground run, and leaves it out when committing.Before:
build_agent_v0_default()returned a default that included platform tools, the authoring skill, and sandbox elevation. Every inspect response returned that enriched default. The commit path had to strip it out manually (or didn't, and it leaked in).After:
build_agent_v0_default()is bare.fetch_simple_applicationattachesadditional_context.playground_build_kit.agent_template_overlayto the response. The frontend'sapplyBuildKitOverlaydoes a deep-merge of the overlay into the run config. The commit reads only persisted entity parameters, so the overlay is excluded for free.The overlay shape:
{ "tools": [ { "type": "platform", "op": "<op_name>" }, { "@ag.embed": { "@ag.references": { "workflow": { "slug": "__ag__..." } } } } ], "skills": [{ "@ag.embed": { "@ag.references": { "workflow": { "slug": "__ag__getting_started_with_agenta" } } } }], "sandbox": { "permissions": { "write_files": "allow", "execute_code": "allow" } } }Key files:
api/oss/src/apis/fastapi/applications/overlay.py(new) —build_agent_template_overlay()assembles the overlay fromPLATFORM_OPS+ reserved static workflow slugs + the authoring skill slug.api/oss/src/apis/fastapi/applications/router.py—fetch_simple_applicationattaches the overlay; create/edit/commit paths do not.services/oss/src/agent/schemas.py— revertedbuild_agent_v0_default()to bare (no platform tools, no skills, no sandbox elevation).web/packages/agenta-playground/src/state/execution/agentRequest.ts—applyBuildKitOverlay(deep-merge objects, identity-merge lists), called inbuildAgentRequestonly when the kit is enabled.web/packages/agenta-entity-ui/.../AgentTemplateControl.tsx— read-only "Playground build kit" panel with an enable/disable toggle and "Removed on commit" tag.Out of scope: collapsible advanced-drawer sections (Change 1 from the design) — ships separately.
Scope / risk
The overlay is computed fresh on every inspect call from static sources (
PLATFORM_OPS,_STATIC_WORKFLOWS, the getting-started slug). It is never stored. A stale catalog entry would show in the overlay without affecting committed revisions.The merge in
applyBuildKitOverlayis shallow for top-level keys and identity-merge for lists. That means the overlay'stoolslist replaces (not appends to) a run'stoolsif the overlay is applied. Review whether deep-merge vs. replace is the right semantic fortoolsin edge cases where the user has also configured tools.The frontend toggle (
buildKitEnabled) defaults on. There is no server-side flag; disabling it is a client session state.Tests
api/oss/tests/pytest/unit/applications/test_build_kit_overlay.py(new): overlay builder shape; inspect response carriesadditional_context.playground_build_kit.services/oss/tests/pytest/unit/agent/test_default_agent_template.py: published default is bare across builtin, inspect schema, and catalog.web/packages/agenta-playground/tests/unit/agentRequest.test.ts: kit-on merges overlay, kit-off sends bare config, commit excludes the kit, applier never mutates the input.Codex-reported results: API overlay test 3/3, services default-agent test 5/5, playground vitest 148/148. Not independently re-run in CI on this branch yet.
How to QA
Prerequisites: local dev stack (
run.sh --oss --devor--ee --dev).Steps:
Expected result: Overlay present on run, absent on commit. Toggle removes it from runs.
Automated tests:
Edge cases: If the user has manually configured tools on their agent before the overlay merges, check that the commit writes only the user-configured tools and not the overlay tools.
https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc