Skip to content

Fix: Fix revision commit endpoints silently discard unknown fields in…#4345

Merged
junaway merged 12 commits into
Agenta-AI:release/v0.100.2from
justin212407:revisions-commit-endpoint-issue
May 25, 2026
Merged

Fix: Fix revision commit endpoints silently discard unknown fields in…#4345
junaway merged 12 commits into
Agenta-AI:release/v0.100.2from
justin212407:revisions-commit-endpoint-issue

Conversation

@justin212407
Copy link
Copy Markdown

@justin212407 justin212407 commented May 15, 2026

Summary

What changed: Set model_config = ConfigDict(extra="forbid") on every *RevisionData DTO so revision-commit endpoints reject unknown top-level fields under data instead of silently dropping them. Pydantic now returns HTTP 422 naming the offending field.

Strict configuration applied to all six revision-commit domains:

Domain API DTO SDK DTO
workflows inherited via BaseWorkflowRevisionData sdks/python/agenta/sdk/models/workflows.pyWorkflowRevisionData
applications api/oss/src/core/applications/dtos.pyApplicationRevisionData sdks/python/agenta/sdk/models/workflows.pyApplicationRevisionData
evaluators api/oss/src/core/evaluators/dtos.pyEvaluatorRevisionData sdks/python/agenta/sdk/models/workflows.pyEvaluatorRevisionData
testsets api/oss/src/core/testsets/dtos.pyTestsetRevisionData sdks/python/agenta/sdk/models/testsets.pyTestsetRevisionData
queries api/oss/src/core/queries/dtos.pyQueryRevisionData
environments api/oss/src/core/environments/dtos.pyEnvironmentRevisionData

Although Pydantic v2 inherits model_config from a strict parent, the explicit declarations on ApplicationRevisionData and EvaluatorRevisionData make the contract grep-friendly per domain.

Why: Fixes #4315. POST /{applications,workflows,evaluators,testsets,queries,environments}/revisions/commit used to accept unknown top-level fields inside data (e.g. the legacy ag_config wrapper) and silently discard them while returning HTTP 200 with count: 1. Users got a false success signal and lost their payload.

Additional changes

  • Removed _normalize_configuration_for_legacy_paths from api/oss/src/core/embeds/service.py. That helper injected a top-level configuration key into embed-resolution payloads, which extra="forbid" would have rejected at WorkflowsService.resolve_workflow_revision. Legacy configuration.parameters.* shapes are no longer supported — the canonical parameters.* path is the only one.

Behavior change

  • Before: data: {"ag_config": {...}} → HTTP 200, count: 1, stored data: {}.
  • After: same payload → HTTP 422 with ag_config named in the validation error.

This is a breaking change for any client still sending stale envelope shapes. Detection is straightforward: callers match on the 422 and migrate.

Testing

New SDK unit testssdks/python/oss/tests/pytest/unit/test_revision_data_extra_forbid.py:

  • WorkflowRevisionData accepts known fields, rejects ag_config and other unknowns.
  • TestsetRevisionData accepts testcase_ids, rejects csvdata.

New acceptance testsapi/oss/tests/pytest/acceptance/test_revision_commit_extra_forbid.py:

  • One test per domain, covering all six /revisions/commit endpoints.
  • Each test creates the parent artifact + variant, then POSTs an unknown top-level data field and asserts the response is 422 with the offending field named in the error body.

Verified locally with curl:

{
  "detail": [
    {
      "type": "extra_forbidden",
      "loc": ["body", "application_revision_commit", "data", "ag_config"],
      "msg": "Extra inputs are not permitted",
      "input": { "prompt": { "messages": [{"role": "system", "content": "sys"}] } }
    }
  ]
}

Valid payloads with data: {"parameters": {...}} continue to return HTTP 200 with count: 1.

Related issues

… data

Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
Copilot AI review requested due to automatic review settings May 15, 2026 23:08
@vercel
Copy link
Copy Markdown

vercel Bot commented May 15, 2026

@justin212407 is attempting to deploy a commit to the agenta projects Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label May 15, 2026
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 15, 2026

CLA assistant check
All committers have signed the CLA.

@dosubot dosubot Bot added Backend Bug Report Something isn't working python Pull requests that update Python code tests labels May 15, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e78d860-a12d-43b4-9d17-f439f98a4123

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR enforces strict Pydantic validation across revision data models by adding model_config = ConfigDict(extra="forbid") to WorkflowRevisionData, QueryRevisionData, and TestsetRevisionData, adds WorkflowRevisionCommitData, and updates workflow DTO data fields to use the new base/commit types.

Changes

Strict validation enforcement

Layer / File(s) Summary
SDK model strict validation contract
sdks/python/agenta/sdk/models/workflows.py
WorkflowRevisionData adds model_config = ConfigDict(extra="forbid") to reject unknown fields during SDK-level validation.
API DTO enforcement across revision types
api/oss/src/core/workflows/dtos.py, api/oss/src/core/queries/dtos.py, api/oss/src/core/testsets/dtos.py
Each API DTO file imports ConfigDict and configures its respective RevisionData model (WorkflowRevisionData, QueryRevisionData, TestsetRevisionData) with extra="forbid". The workflows DTO also defines WorkflowRevisionCommitData (forbids extra fields) and updates WorkflowRevisionCommit.data to use the stricter commit type.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 60.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly relates to the main change: fixing revision commit endpoints to reject unknown fields instead of silently discarding them.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly explains the changes: adding strict validation via ConfigDict(extra='forbid') to multiple RevisionData DTOs across six domains to prevent silent data loss when unknown fields are sent.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens validation for workflow revision data payloads so unknown fields are rejected instead of silently discarded during commit operations.

Changes:

  • Adds extra="forbid" to SDK WorkflowRevisionData.
  • Adds a server-side strict WorkflowRevisionData wrapper.
  • Adds an application revision regression test for stale ag_config payloads returning 422.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
sdks/python/agenta/sdk/models/workflows.py Makes SDK workflow revision data reject unknown fields.
api/oss/src/core/workflows/dtos.py Makes server workflow revision data reject unknown fields.
api/oss/tests/pytest/acceptance/applications/test_application_variants_and_revisions.py Adds an acceptance test for application revision commit validation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sdks/python/agenta/sdk/models/workflows.py
Comment thread api/oss/src/core/workflows/dtos.py Outdated
@justin212407
Copy link
Copy Markdown
Author

@jp-agenta Could you review these changes once you are free?

Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels May 24, 2026
Copilot AI review requested due to automatic review settings May 24, 2026 14:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment thread api/oss/src/core/workflows/dtos.py
Comment thread api/oss/src/core/testsets/dtos.py
Comment thread api/oss/src/core/queries/dtos.py
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ee7999e1-9f30-4a60-85ac-4edb138434e3

📥 Commits

Reviewing files that changed from the base of the PR and between bb3ce13 and 41777ce.

📒 Files selected for processing (1)
  • api/oss/src/core/workflows/dtos.py

Comment thread api/oss/src/core/workflows/dtos.py Outdated
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
Copilot AI review requested due to automatic review settings May 25, 2026 12:21
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels May 25, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

api/oss/src/core/testsets/dtos.py:160

  • extra="forbid" on TestsetRevisionData changes request validation behavior for /testsets/revisions/commit (unknown keys under data will now return 422 instead of being ignored). Please add/adjust acceptance tests to cover this new rejection path, so clients get a clear, stable error contract and we don’t regress back to silently dropping fields.
class TestsetRevisionData(BaseModel):
    model_config = ConfigDict(extra="forbid")

    testcase_ids: Optional[List[UUID]] = None
    testcases: Optional[List[Testcase]] = None

api/oss/src/core/queries/dtos.py:146

  • With extra="forbid" on QueryRevisionData, unknown keys under data will now be rejected with 422 for /queries/revisions/commit. Add acceptance test coverage for this failure mode (assert status 422 and that the response mentions the offending key) to lock in the intended behavior and avoid future silent-drop regressions.
class QueryRevisionData(BaseModel):
    model_config = ConfigDict(extra="forbid")

    formatting: Optional[Formatting] = None
    filtering: Optional[Filtering] = None
    windowing: Optional[Windowing] = None

Comment thread api/oss/src/core/workflows/dtos.py
Comment thread api/oss/src/core/workflows/dtos.py Outdated
Comment thread api/oss/src/core/testsets/dtos.py
Comment thread api/oss/src/core/queries/dtos.py
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels May 25, 2026
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
Copilot AI review requested due to automatic review settings May 25, 2026 12:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

sdks/python/agenta/sdk/models/workflows.py:135

  • The PR description claims a new acceptance test test_commit_application_revision_rejects_unknown_data_field was added, but it is not present in api/oss/tests/pytest/acceptance/applications/test_application_variants_and_revisions.py in this branch. Please add the described test (or update the PR description) so the stricter extra="forbid" behavior is protected against regressions.
class WorkflowRevisionData(BaseModel):
    model_config = ConfigDict(extra="forbid")

    uri: Optional[str] = None

    url: Optional[str] = None
    headers: Optional[Dict[str, Union[str, Reference]]] = None

    runtime: Optional[Literal["python", "typescript", "javascript"]] = None
    script: Optional[str] = None

    schemas: Optional[JsonSchemas] = None

    parameters: Optional[Data] = None

api/oss/src/core/testsets/dtos.py:160

  • This changes TestsetRevisionData validation to reject unknown keys (extra="forbid"), which will make /testsets/revisions/commit (and any other endpoints consuming this DTO) return 422 for stale payload shapes. Please add an acceptance/regression test that exercises the commit endpoint with an unknown field and asserts the 422 error contains the offending key, to prevent reintroducing silent drops.
class TestsetRevisionData(BaseModel):
    model_config = ConfigDict(extra="forbid")

    testcase_ids: Optional[List[UUID]] = None
    testcases: Optional[List[Testcase]] = None

api/oss/src/core/workflows/dtos.py:71

  • The PR description says extra="forbid" is set on WorkflowRevisionData in this file, but the only explicit ConfigDict(extra="forbid") is on WorkflowRevisionCommitData (which is not used by WorkflowRevisionCommit.data). If the goal is to make commit endpoints reject unknown fields via the server-side DTO, please apply the config on the actual WorkflowRevisionData type used by the commit/request models (or update the description to match the implementation).
class WorkflowRevisionData(BaseWorkflowRevisionData):
    pass


WorkflowRevisionCommitData = WorkflowRevisionData


class WorkflowRevisionCommitData(BaseWorkflowRevisionData):
    model_config = ConfigDict(extra="forbid")

Comment thread api/oss/src/core/testsets/dtos.py
Comment thread api/oss/src/core/queries/dtos.py
Comment thread api/oss/src/core/workflows/dtos.py
Signed-off-by: justin212407 <charlesjustin2124@gmail.com>
@justin212407
Copy link
Copy Markdown
Author

this one got a little messy with all the commits opening the new one with the right changes.

@junaway junaway reopened this May 25, 2026
@justin212407
Copy link
Copy Markdown
Author

Hey I have made the new PR here - #4415. Please check this out.

Copilot AI review requested due to automatic review settings May 25, 2026 14:29
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels May 25, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Comment thread api/oss/src/core/workflows/dtos.py Outdated
Comment thread api/oss/src/core/workflows/dtos.py
Although Pydantic v2 inherits model_config from WorkflowRevisionData,
declaring extra="forbid" explicitly on ApplicationRevisionData and
EvaluatorRevisionData (both API and SDK) makes the strict-validation
contract grep-friendly and matches the per-domain expectation in Agenta-AI#4315.

Also drop the now-unused ConfigDict import from workflows/dtos.py;
WorkflowRevisionData inherits its strict config from the SDK base.

Picked up from PR Agenta-AI#4415.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@junaway junaway changed the base branch from main to release/v0.100.2 May 25, 2026 14:43
@junaway junaway merged commit 702b842 into Agenta-AI:release/v0.100.2 May 25, 2026
11 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Bug Report Something isn't working python Pull requests that update Python code size:M This PR changes 30-99 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revisions commit endpoints silently discard unknown fields in data

5 participants