feat(api): validate the agent template on commit and test_run - #5104
Closed
mmabrouk wants to merge 1 commit into
Closed
feat(api): validate the agent template on commit and test_run#5104mmabrouk wants to merge 1 commit into
mmabrouk wants to merge 1 commit into
Conversation
dosubot
Bot
added
the
size:XL
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 1768 files, which is 1618 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (258)
📒 Files selected for processing (1768)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
|
mmabrouk
commented
Jul 7, 2026
| ] | ||
|
|
||
|
|
||
| def _tool_errors(tools: List[Any]) -> List[Dict[str, Any]]: |
Member
Author
There was a problem hiding this comment.
The shape-vs-completeness split, for review: skills and mcps drop the incompleteness error types (missing / too-short / pattern) so playground drafts stay committable, but TYPED tool entries (type in the config union) validate strictly with no such tolerance. That asymmetry is deliberate: typed tool configs only come from discovery output or the agent's own commit, both of which emit complete entries, while the playground commits tools in the loose function-shape that passes through the legacy branch. If partial typed-tool drafts ever become a real surface, apply the same ignore set to _tool_errors.
mmabrouk
commented
Jul 7, 2026
| return None | ||
|
|
||
|
|
||
| def _harness_provider_errors(agent: Dict[str, Any]) -> List[Dict[str, Any]]: |
Member
Author
There was a problem hiding this comment.
Conscious call: harness_allows_provider('claude', ...) lists Anthropic only, so Claude-via-Bedrock/Vertex would be rejected here. That matches the capability table (the single source of truth, also used by the model picker), but if Bedrock/Vertex Claude becomes a supported config the table is the place to extend, not this rule. Also deliberate: a bare model id with no determinable provider passes (Claude's implicit Anthropic default), so {harness: claude, llm: {model: "gpt-4o"}} with no provider slips through; the capability table has no model-id-to-provider map to catch it.
mmabrouk
commented
Jul 7, 2026
| # paths) rather than run a broken agent. | ||
| if env.agenta.agent_template.commit_validation: | ||
| try: | ||
| validate_agent_template(resolved.data) |
Member
Author
There was a problem hiding this comment.
test_run's in-memory delta gets the same validation as a real commit, so a bad test-before-commit delta is refused (400 via PlatformToolHandlerRefused) instead of silently running a fallback config. One migration note for both hook points: a PERSISTED legacy-shape agent revision (flat model instead of llm.model) will be refused on its next commit or test_run because the strict schema forbids extras. Known instances are dev data only; AGENTA_AGENT_TEMPLATE_COMMIT_VALIDATION=false is the escape hatch.
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![github-advanced-security[bot]](https://avatars.github.com/in/57789?s=60&v=4){kind=small}
| let last = 0 | ||
| let m: RegExpExecArray | null | ||
| TOKEN_RE.lastIndex = 0 | ||
| while ((m = TOKEN_RE.exec(template)) !== null) { |
| "content-type": "application/json", | ||
| "content-length": Buffer.byteLength(payload), | ||
| }); | ||
| res.end(payload); |
| if (!isPlainObject(cursor[key])) cursor[key] = {}; | ||
| cursor = cursor[key] as Record<string, unknown>; | ||
| } | ||
| cursor[parts[parts.length - 1]] = value; |
dosubot
Bot
added
size:L
Member
Author
|
Closing by Mahmoud's decision: server-side validation does not belong in the workflows service — changes to that surface need a full design pass and CTO sign-off. The interim fix is at the SDK layer instead: typed input schemas on the platform ops (the agent-template shape advertised in the tool schema itself, over MCP and Pi) plus the skill reference files with example requests. Decision recorded in docs/design/agent-workflows/projects/builder-agent-reliability/tools-review/part-3-agenta-skills-sync.md. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Nothing guarded
parameters.agenton the write path. A builder agent could commitharness.kind: "claude"with an OpenAI provider, or a skill entry withslug/contentat the top level, and the commit succeeded silently. The agent then never runs, or silently falls back to a default config, and the model that made the mistake gets no signal to fix it. The strictAgentTemplateSchemaalready existed in the SDK; it was only used to generate the playground editor, never enforced.Changes
New
api/oss/src/core/workflows/agent_validation.pyvalidates the delta-resolved finalparameters.agentincommit_workflow_revision(coversPOST /workflows/revisions/commit, initial creates, and the applications/evaluators services that delegate there) and in thetools.agenta.test_runhandler's in-memory delta.A failing commit now returns a structured 400 a self-remediating model can act on:
{"message": "The agent template configuration failed validation.", "errors": [{"loc": "parameters.agent.skills.0.slug", "msg": "Extra inputs are not permitted", "type": "extra_forbidden"}]}One cross-field rule rides on top:
harness.kind: "claude"requires an Anthropic provider, read from the existing capability table (harness_allows_provider), never re-encoded.The commit path is also a draft surface, so validation splits shape from completeness:
@ag.embedentries pass through untouched; the playground's loose tool shapes (OpenAI{type: "function"}, flat, bare builtin) are tolerated; blank or half-filled skill/MCP drafts are tolerated; wrong shapes (unknown keys, wrong types, invalid typed tool configs) reject. The shipped default template plus the build-kit overlay is pinned committable by a test.Kill switch:
AGENTA_AGENT_TEMPLATE_COMMIT_VALIDATION(default on) inenv.py.Scope / risk
Write-path only: stored revisions are never re-validated. Two conscious calls, flagged inline: a persisted legacy-shape agent revision (flat
modelinstead ofllm.model) will be refused on its next commit or test_run delta (the kill switch is the escape hatch, and only dev data is known to have that shape); Claude via Bedrock/Vertex would be rejected by the pairing rule because the capability table lists Anthropic only. The/applications/revisions/commitroute delegates to the same service so invalid data is blocked there too, but that router lacks the exception decorator and would surface a 500 instead of the structured 400 (pre-existing gap, one-decorator follow-up).Tests
test_agent_template_validation.py(20 tests) covering the validator, both hook points, the kill switch, and the router translation;test_platform_handlers.pyextended with a refusal test.How to QA
Prerequisites: local dev stack, any project API key, an agent app.
Steps:
POST /api/workflows/revisions/commitwithdelta.set.parameters.agent.skills = [{"slug": "bad", "content": "nope"}].delta.set.parameters.agent = {"harness": {"kind": "claude"}, "llm": {"provider": "openai", "model": "gpt-5.5", "connection": {"mode": "agenta"}}}.Expected result: steps 1 and 2 return 400 with
errors[].locnaming the offending fields; step 3 returns 200 with a new revision.Automated tests:
Edge cases: commit the untouched default agent template from the playground (must pass); commit a non-agent workflow (must be unaffected); set
AGENTA_AGENT_TEMPLATE_COMMIT_VALIDATION=falseand repeat step 1 (must pass through).https://claude.ai/code/session_01N2djTMgXnpk84EqtugHDJB