feat(agent): playground build kit (default agent config)#4926
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:
✨ 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 |
|
@Agenta-AI please review tomorrow — DRAFT, do not merge. Specific feedback wanted:
Implemented by Codex (gpt-5.5, xhigh). Change 1 (collapsible drawer sections) intentionally excluded. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
services/oss/tests/pytest/unit/agent/test_default_agent_template.py (1)
59-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror the sandbox-flag assertions for the builtin default.
The test name says every published default, but
execute_codeandwrite_filesare only checked oninspect_default. A regression on the SDK builtin path would still pass this suite.Proposed test tightening
assert builtin_default["tools"] == [] assert "permissions" not in builtin_default["sandbox"] + assert "execute_code" not in builtin_default["sandbox"] + assert "write_files" not in builtin_default["sandbox"] assert "skills" not in builtin_defaultweb/packages/agenta-entities/src/workflow/state/store.ts (1)
1106-1114: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate the overlay with a schema at the inspect boundary.
This only proves the top level is an object, so drift inside
sandbox,tools, orskillswill flow straight into the merge/UI path asAgentTemplate. Please runagent_template_overlaythrough a local Zod schema andsafeParseWithLoggingbefore exposing it from this atom family. Based on coding guidelines, "Keep Zod validation at the API boundary even when using Fern-generated types, because local schemas still detect backend drift" and "UsesafeParseWithLoggingfrom@agenta/entities/sharedfor boundary validation so structured errors are logged without crashing."Source: Coding guidelines
web/packages/agenta-playground/tests/unit/agentRequest.test.ts (1)
285-305: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the real commit source stays bare.
Line 302 checks
workflowMolecule.selectors.configuration("e"), butprepareCommitParametersreadsentity.data?.parameters. This test can still pass if build-kit fields start leaking into the actual commit payload later. Seedover.datawithdata.parametersand assert that object remains unchanged instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 38316881-acc2-4f09-88d6-97cbd6271674
📒 Files selected for processing (20)
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.pydocs/design/agent-workflows/projects/default-agent-config/README.mddocs/design/agent-workflows/projects/default-agent-config/design.mddocs/design/agent-workflows/projects/default-agent-config/research.mddocs/design/agent-workflows/projects/default-agent-config/status.mdservices/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
| 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.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
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."
Source: Coding guidelines
| 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.
🎯 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.
Suggested fix
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)📝 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.
| revision = catalog.retrieve_revision(slug=slug) | |
| if revision and revision.flags and revision.flags.is_skill: | |
| continue | |
| slugs.append(slug) | |
| revision = catalog.retrieve_revision(slug=slug) | |
| if not revision or not revision.flags or revision.flags.is_skill: | |
| continue | |
| slugs.append(slug) |
| additional_context=SimpleApplicationAdditionalContext( | ||
| playground_build_kit=PlaygroundBuildKitContext( | ||
| agent_template_overlay=build_agent_template_overlay(), | ||
| ), | ||
| ) | ||
| if simple_application | ||
| else None, |
There was a problem hiding this comment.
🩺 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.
Source: Path instructions
| function overriddenPermissionKeys( | ||
| userPermissions: Record<string, unknown> | null | undefined, | ||
| overlayPermissions: Record<string, unknown> | null | undefined, | ||
| ): string[] { | ||
| if (!userPermissions || !overlayPermissions) return [] | ||
| return Object.entries(overlayPermissions) | ||
| .filter(([key, overlayValue]) => { | ||
| if (!(key in userPermissions)) return false | ||
| return stableString(userPermissions[key]) !== stableString(overlayValue) | ||
| }) | ||
| .map(([key]) => key) |
There was a problem hiding this comment.
🎯 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.
Suggested fix
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)
}📝 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.
| function overriddenPermissionKeys( | |
| userPermissions: Record<string, unknown> | null | undefined, | |
| overlayPermissions: Record<string, unknown> | null | undefined, | |
| ): string[] { | |
| if (!userPermissions || !overlayPermissions) return [] | |
| return Object.entries(overlayPermissions) | |
| .filter(([key, overlayValue]) => { | |
| if (!(key in userPermissions)) return false | |
| return stableString(userPermissions[key]) !== stableString(overlayValue) | |
| }) | |
| .map(([key]) => key) | |
| function overriddenPermissionKeys( | |
| userPermissions: Record<string, unknown> | null | undefined, | |
| overlayPermissions: Record<string, unknown> | null | undefined, | |
| ): string[] { | |
| if (!overlayPermissions) return [] | |
| return Object.entries(overlayPermissions) | |
| .filter( | |
| ([key, overlayValue]) => | |
| stableString(userPermissions?.[key]) !== stableString(overlayValue), | |
| ) | |
| .map(([key]) => key) | |
| } |
| const agentTemplateOverlay = useAtomValue( | ||
| useMemo(() => workflowAgentTemplateOverlayAtomFamily(revisionId ?? ""), [revisionId]), | ||
| ) | ||
| const [buildKitEnabled, setBuildKitEnabled] = useAtom( | ||
| useMemo(() => workflowBuildKitEnabledAtomFamily(revisionId ?? ""), [revisionId]), | ||
| ) | ||
| const [buildKitExpanded, setBuildKitExpanded] = useState(true) |
There was a problem hiding this comment.
🎯 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.
Suggested fix
+ 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])📝 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 agentTemplateOverlay = useAtomValue( | |
| useMemo(() => workflowAgentTemplateOverlayAtomFamily(revisionId ?? ""), [revisionId]), | |
| ) | |
| const [buildKitEnabled, setBuildKitEnabled] = useAtom( | |
| useMemo(() => workflowBuildKitEnabledAtomFamily(revisionId ?? ""), [revisionId]), | |
| ) | |
| const [buildKitExpanded, setBuildKitExpanded] = useState(true) | |
| const agentTemplateOverlay = useAtomValue( | |
| useMemo(() => workflowAgentTemplateOverlayAtomFamily(revisionId ?? ""), [revisionId]), | |
| ) | |
| const [buildKitEnabled, setBuildKitEnabled] = useAtom( | |
| useMemo(() => workflowBuildKitEnabledAtomFamily(revisionId ?? ""), [revisionId]), | |
| ) | |
| const [buildKitExpanded, setBuildKitExpanded] = useState(true) | |
| 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, buildKitEnabled], | |
| ) | |
| const cancelSection = useCallback(() => { | |
| if (sectionSnapshot.current) onChange(sectionSnapshot.current) | |
| if (openSection === "advanced" && buildKitSnapshot.current != null) { | |
| setBuildKitEnabled(buildKitSnapshot.current) | |
| } | |
| setOpenSection(null) | |
| }, [onChange, openSection, setBuildKitEnabled]) |
| {hasBuildKitOverlay ? ( | ||
| <div className="rounded border border-solid border-[var(--ag-c-EAEFF5,#eaeff5)] bg-[#fcfcfa]"> | ||
| <button | ||
| type="button" | ||
| onClick={() => setBuildKitExpanded((open) => !open)} | ||
| className="flex w-full cursor-pointer items-center gap-2 border-0 bg-transparent px-3 py-2.5 text-left" | ||
| > | ||
| <Wrench size={15} className="text-[var(--ag-c-586673,#586673)]" /> | ||
| <span className="text-[13px] font-medium">Playground build kit</span> | ||
| <span className="ml-auto inline-flex items-center gap-1.5 text-[11px] text-[var(--ag-c-586673,#586673)]"> | ||
| <span className="h-1.5 w-1.5 rounded-full bg-[#d97706]" /> | ||
| Removed on commit | ||
| </span> | ||
| <span | ||
| onClick={(e) => e.stopPropagation()} | ||
| className="inline-flex items-center" | ||
| > | ||
| <Switch | ||
| size="small" | ||
| checked={buildKitEnabled} | ||
| onChange={setBuildKitEnabled} | ||
| disabled={disabled} | ||
| /> | ||
| </span> | ||
| <CaretRight | ||
| size={14} | ||
| className={cn( | ||
| "text-[var(--ag-c-97A4B0,#97a4b0)] transition-transform", | ||
| buildKitExpanded && "rotate-90", | ||
| )} | ||
| /> | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx'
# Show the relevant region with line numbers.
sed -n '1618,1692p' "$file"
# Map the component structure around the snippet.
grep -nE '<button|</button>|<Switch|onClick=|stopPropagation' "$file" | sed -n '1,120p'Repository: Agenta-AI/agenta
Length of output: 5513
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.
1 participant
Summary
Implements the playground build kit from the #4917 design: an agent-template overlay the backend serves read-only on the inspect response, the frontend applies to a kit-on playground run, and that never reaches a commit or a deployed run. The published default agent goes bare; the platform tools, authoring skill, and elevated sandbox now ride the overlay instead of the stored config.
This covers the full "Change set, by layer" from the design except Change 1 (collapsible advanced-drawer sections), which ships separately.
Implements the #4917 design; DRAFT for review tomorrow, do not merge.
What changed (file-by-file, from the Codex implementation summary)
Backend
api/oss/src/apis/fastapi/applications/models.py— addedadditional_context.playground_build_kit.agent_template_overlaytoSimpleApplicationResponse.api/oss/src/apis/fastapi/applications/router.py— populates the overlay only infetch_simple_application(read path), never in create/edit/commit.api/oss/src/apis/fastapi/applications/overlay.py(new) — deterministic overlay builder:toolsfromPLATFORM_OPS({type: platform, op}) plus reserved-slug static workflows as@ag.embedreferences;skillsas the@ag.embedof the authoring slug;sandbox.permissionsbuild elevation.services/oss/src/agent/schemas.py— reverted the enrichment sobuild_agent_v0_default()is bare (no authoring skill, no sandbox elevation).Frontend
web/packages/agenta-entities/src/workflow/api/api.ts,state/store.ts— session atoms for the overlay andbuildKitEnabled(default on).web/packages/agenta-playground/src/state/execution/agentRequest.ts—applyBuildKitOverlay(deep-merge object fields, identity-merge list fields), called inbuildAgentRequestonly when the kit is on, on a throwaway run copy.web/packages/agenta-entities/src/workflow/state/commit.ts— comment confirming commit reads only persisted entity parameters (overlay excluded for free).web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx— read-only "Playground build kit" section with enable/disable toggle, "Removed on commit" tag, locked overlay rendering, and a sandbox override hint on overridden user controls.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, catalog.web/packages/agenta-playground/tests/unit/agentRequest.test.ts— kit-on merges overlay (deep + identity merge), kit-off sends bare config, commit excludes the kit, applier never mutates the input.Verification (reported by Codex)
ruff format+ruff check --fix: passed.cd web && pnpm lint-fix: passed.@agenta/entities,@agenta/playground,@agenta/entity-ui): passed.git diff --check: passed.Notes for the reviewer
https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc