feat(agent): playground build kit (default agent config)#4929
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. 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
| 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
| additional_context?: { | ||
| playground_build_kit?: { | ||
| agent_template_overlay?: Record<string, unknown> | null | ||
| } | null | ||
| } | null |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline web/packages/agenta-entities/src/workflow/api/api.ts --view expanded
rg -n -C4 'inspectWorkflow|safeParseWithLogging|additional_context|playground_build_kit|agent_template_overlay' \
web/packages/agenta-entities/src/workflow/api/api.ts \
web/packages/agenta-entities/src/workflow/state/store.tsRepository: Agenta-AI/agenta
Length of output: 27225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' web/packages/agenta-entities/src/workflow/api/api.ts
printf '\n--- SNIP ---\n'
sed -n '220,560p' web/packages/agenta-entities/src/workflow/api/api.ts
printf '\n--- SNIP ---\n'
sed -n '560,620p' web/packages/agenta-entities/src/workflow/api/api.tsRepository: Agenta-AI/agenta
Length of output: 21474
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '469,560p' web/packages/agenta-entities/src/workflow/api/api.ts
printf '\n--- SCHEMA ---\n'
sed -n '1,220p' web/packages/agenta-entities/src/workflow/schema.tsRepository: Agenta-AI/agenta
Length of output: 3073
Validate the inspect response before returning it. inspectWorkflow returns raw response.data, so additional_context.playground_build_kit.agent_template_overlay can reach workflowAgentTemplateOverlayAtomFamily without local schema validation; wrap this in safeParseWithLogging first.
Source: Coding guidelines
| const withBuildKitOverlay = ( | ||
| parameters: Record<string, unknown>, | ||
| overlay: AgentTemplate | null, | ||
| enabled: boolean, | ||
| ): Record<string, unknown> => { | ||
| if (!enabled || !overlay || !isRecord(parameters.agent)) return parameters | ||
| return { | ||
| ...parameters, | ||
| agent: applyBuildKitOverlay(parameters.agent as AgentTemplate, overlay), | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply the overlay to bare agent templates too.
withAgentRunDefaults() already supports both {agent: ...} and bare templates, but this helper no-ops unless parameters.agent exists. That skips the build kit entirely for the bare-template shape this file already handles, which is risky now that the backend default was reverted to a bare template. Based on PR objectives, the bare default is part of this rollout.
Proposed fix
const withBuildKitOverlay = (
parameters: Record<string, unknown>,
overlay: AgentTemplate | null,
enabled: boolean,
): Record<string, unknown> => {
- if (!enabled || !overlay || !isRecord(parameters.agent)) return parameters
- return {
- ...parameters,
- agent: applyBuildKitOverlay(parameters.agent as AgentTemplate, overlay),
- }
+ if (!enabled || !overlay) return parameters
+ if (isRecord(parameters.agent)) {
+ return {
+ ...parameters,
+ agent: applyBuildKitOverlay(parameters.agent as AgentTemplate, overlay),
+ }
+ }
+ return applyBuildKitOverlay(parameters as AgentTemplate, overlay) as Record<string, unknown>
}📝 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 withBuildKitOverlay = ( | |
| parameters: Record<string, unknown>, | |
| overlay: AgentTemplate | null, | |
| enabled: boolean, | |
| ): Record<string, unknown> => { | |
| if (!enabled || !overlay || !isRecord(parameters.agent)) return parameters | |
| return { | |
| ...parameters, | |
| agent: applyBuildKitOverlay(parameters.agent as AgentTemplate, overlay), | |
| } | |
| const withBuildKitOverlay = ( | |
| parameters: Record<string, unknown>, | |
| overlay: AgentTemplate | null, | |
| enabled: boolean, | |
| ): Record<string, unknown> => { | |
| if (!enabled || !overlay) return parameters | |
| if (isRecord(parameters.agent)) { | |
| return { | |
| ...parameters, | |
| agent: applyBuildKitOverlay(parameters.agent as AgentTemplate, overlay), | |
| } | |
| } | |
| return applyBuildKitOverlay(parameters as AgentTemplate, overlay) as Record<string, unknown> | |
| } |
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
Application overlay + agent-template schema/controls, workflow entity state wiring, and the AgentTemplateControl drawer UI. Design docs live in the agent-design-docs branch. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
…onse, not agent-service inspect
…and parse (#4917) The overlay emitted each @ag.embed reference without an @ag.selector, so the server-side resolver inlined the whole revision.data ({uri, parameters:{skill|tool:...}}) instead of the flat inline value. AgentTemplate.from_params then raised HTTP 500 'Unsupported tool configuration shape' on the tools field (and would have failed skills too). Add the canonical selector: parameters.skill for the authoring skill, parameters.tool for each reserved static tool. Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
mmabrouk
force-pushed
the
feat/build-kit-4917-v2
branch
from
c1d6561 to
332c5d8
Compare
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