Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Invoke validation

Implemented. Validation lands at the SDK resolver boundary (see `status.md`).

The agent service invoke endpoint (`POST {host}/services/agent/v0/invoke`) did not tell a
caller when the request was malformed. Send `references` with no revision, or send
`data.revision` one level too shallow, and the service silently ran a seeded default
(`pi_core` / `gpt-5.5`) and then 500'd. The caller got an opaque late error instead of "your
request was shaped wrong, here is the right shape."

This project reframes that as a validation problem. The boundary validates the request up front
and, when it cannot resolve to a config, returns a clear 400 that names the two valid ways to
invoke an application:

1. Provide configuration inline (`data.parameters`).
2. Provide a revision: either a complete revision nested correctly (`data.revision = {"data":
...}`), or a resolvable reference that pins one committed config (a variant, an environment, or
a revision, not a bare application).

## Files

- `context.md` — the symptom (malformed invoke -> silent default -> late 500), why it cost the
lab time, and why validation (not self-hydration) is the right lens. Goals and non-goals.
- `research.md` — the mechanism with `file:line` citations: the resolver requires a double-nested
revision (`resolver.py:150`), the agent's seeded default disables reference hydration
(`utils.py:285-287`, gate at `resolver.py:573-577`), the envelope ignores unknown fields
(`workflows.py:237`, `:296`), the product path pre-hydrates the reference so it works
(`service.py:745-751`), the reference-family validator that already raises 4xx
(`resolver.py:69-98`), the three reference-kind results, and the live reproduction.
- `plan.md` — the shipped validation: Rules A (revision nested right) and B (reference must be
resolvable, not a bare application), raised as a clear 400 at the shared resolver boundary,
consistent across agent / completion / chat. Scope limits (no blanket forbid, no OpenAPI, no
"nothing to run" rule so completion / chat do not regress) and the test matrix.
- `status.md` — implemented; decisions locked and open questions resolved.

## Status

Implemented at the SDK resolver boundary (`_validate_resolvable_config`). Supersedes and merges
the earlier `harden-invoke` decision and the `silent-fallback` / `invoke-contract` threads under
`docs/design/agent-workflows/scratch/console/builder-kit/`. See `status.md`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# Context: validate the invoke request instead of silently defaulting

## The symptom

A caller sends a malformed request to the agent service invoke endpoint
(`POST {host}/services/agent/v0/invoke`). The endpoint does not tell the caller the request
is malformed. It silently runs a seeded default agent (`pi_core` / `gpt-5.5`), and then that
default 500s a few steps later because `gpt-5.5` needs a provider prefix. The caller sees a
late, confusing 500. Nothing points back at the real cause: the request was shaped wrong.

Two malformed shapes both hit this trap:

- **References only, no revision.** The caller passes `references` (for example an
`application_revision` ref) and expects the service to fetch and run the committed config.
It does not. The service runs the seeded default and 500s.
- **A revision nested one level too shallow.** The caller passes
`data.revision = <revision.data>` (the bare revision fields), when the resolver requires the
double-nested `data.revision = {"data": <revision.data>}`. The wrong nesting is ignored, the
seeded default runs, and it 500s.

Both are the same failure from the caller's point of view: I sent config, the run ignored it,
and I got an opaque 500 with the wrong model in the trace.

## Why this cost the lab time

The agent-creation lab drives the low-level service endpoint directly (it does not go through
the product API). During the lab, guessing the right invoke shape was slow and expensive
because a wrong guess did not fail loudly. It returned a 200-then-500 with a default config in
the resolved trace, which looks like a runtime bug in the agent rather than a bad request. The
operator had to reverse-engineer the exact nesting from the resolver source to find the one
working shape. A single clear 4xx at the boundary would have replaced that whole investigation.

## Why validation is the right lens

An earlier framing called this a "self-hydration" gap: the service should fetch the reference
itself the way completion and chat do. That fix is real and worth doing (see the sibling
finding on the seeded-default hydration gate), but it is not the frame the user wants for this
project. The user's frame is simpler and more durable:

> This is a validation problem. The endpoint should check the request up front and, when the
> request cannot resolve to a config, return a clear error that names the valid ways to call
> the application.

Under that frame there are exactly two valid ways to invoke — there is no legitimate empty or
default intent, so the caller either gives a config or specifies a revision:

1. **Provide configuration inline** (`data.parameters = {"agent": {...}}`).
2. **Provide a revision**: either a complete revision nested correctly (`data.revision = {"data":
...}`), or a resolvable reference the service can fetch a specific committed config from.

And a key rule inside option 2: a bare `application` reference is not enough. An application
holds many variants and revisions, so `application` alone cannot pin one config. A resolvable
reference needs at least a variant, an environment, or a revision. The endpoint validates that
too.

The goal: a caller that sends the data the wrong way gets a precise error that explains the
right way, instead of a silent default followed by a late 500.

## Goals

- Turn a malformed invoke into a clear 400 at the boundary, before any run starts.
- Make the error name the two valid call shapes and, for references, name the missing
identity (variant, environment, or revision).
- Validate that a supplied revision is nested correctly.
- Keep the behavior consistent across the agent, completion, and chat services where the
contract is shared.

## Non-goals

- **Not OpenAPI.** The decision to keep the services' OpenAPI off stands (a bare `Optional[dict]`
parameters field renders as an opaque, misleading schema; `/inspect` is the live contract).
See the superseded `harden-invoke` decision.
- **Not blanket `extra="forbid"` on the whole envelope.** The user rejected locking the request
shape down wholesale. Validation should fail loud with a good message on the load-bearing
fields (is there a resolvable target, is a supplied revision nested right), not reject every
unknown field.
- **Not a rewrite of reference resolution.** This project validates the request shape at the
boundary. Whether the service also self-hydrates a bare reference (the seeded-default gate) is
tracked as related work and can land alongside, but the validation is the primary deliverable.

## Relationship to earlier notes

This project supersedes and merges the earlier `harden-invoke` decision and the `silent-fallback`
and `invoke-contract` threads under
`docs/design/agent-workflows/scratch/console/builder-kit/`. Those converged on "add strict
validation with clear errors, keep OpenAPI off." This workspace is the durable home for that
conclusion, reframed around request validation and the two valid call shapes.
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# Plan: validate the invoke request at the boundary

## The core idea

Validate the invoke request before any run starts. When the request cannot resolve to a config,
return a 400 whose message names the two valid ways to call an application. Do this in the shared
SDK resolver boundary that already validates reference families, so the behavior is consistent
across the agent, completion, and chat services.

The rule the boundary enforces: **a well-formed invoke supplies its config in one of two ways.**
There is no legitimate empty or default intent — the caller either gives a config or specifies a
revision.

1. **Inline configuration:** `data.parameters = {"agent": {...}}`.
2. **A revision.** Either:
- a complete revision, nested correctly: `data.revision = {"data": <revision.data>}`; or
- a resolvable reference that pins one committed config: a variant, an environment, or a
revision (`latest` is fine), not a bare application.

If neither holds, the request is malformed and gets a clear error instead of a silent default and
a late 500.

## What "validate" means, precisely

The check runs at the resolver boundary, right where `_validate_executable_reference_families`
already runs (`sdks/python/agenta/sdk/middlewares/running/resolver.py:69-98`). It answers one
question: can this request resolve to a caller-intended config? It raises `bad_request` (400) with
a specific message when it cannot. It ships as a sibling validator, `_validate_resolvable_config`,
called at the top of `ResolverMiddleware.__call__`, so it does not disturb the family check or the
`resolve_references_with_info` path.

### Rule A: a revision, if present, must be nested correctly

If `data.revision` is present, it must be the double-nested shape the resolver reads
(`data.revision["data"]`, `resolver.py:150`). If `data.revision` is a dict but has no `data`
key, the caller almost certainly sent the bare revision fields one level too shallow. Reject
with a message that shows the required shape:

> `data.revision` must be nested as `{"data": {...}}` (the revision's own fields go under
> `data`). You sent the fields at the top level. Wrap them: `data.revision = {"data":
> <revision fields>}`.

This turns the silent single-nested failure (live result: HTTP 500, `gpt-5.5`) into an
immediate, self-explaining 400.

### Rule B: a reference, if it is the intended target, must be resolvable

If the request supplies `references` but no `data.revision` and no inline `data.parameters`,
then the reference is the intended config source. Validate that the reference can pin one
committed config:

- A bare `application` reference (no variant, no environment, no revision) is not resolvable.
An application has many revisions; nothing selects which one to run. Reject with:

> A bare `application` reference cannot select a config. An application has many variants and
> revisions. Provide one of: an `application_variant` reference, an `environment` reference,
> or an `application_revision` reference.

- The same rule applies per family (workflow, evaluator) using their variant/revision members.

This reuses the family grouping already in `_validate_executable_reference_families`. That
function proves the pattern: it groups references into `workflow` / `application` / `evaluator`
families and raises a clear `bad_request`. Rule B extends it from "exactly one family" to "and
that family names a resolvable target."

### No "nothing to run" rule for the empty body

An earlier draft added a Rule C that rejected a request with no revision, no reference, and no
inline parameters. It is dropped. An empty request expresses no config intent, and config can
still arrive from the running context or a pre-installed handler, so rejecting it at the shared
boundary would regress completion and chat (and the local workflow path). The validator therefore
rejects only an EXPRESSED but unresolvable intent: a present single-nested `data.revision`
(Rule A) or references that pin nothing (Rule B). That covers the two shapes the lab actually hit
— a wrong nesting and a references-only call with no committed target — while leaving the empty
body alone.

## Scope of the strictness (what we are NOT doing)

- **No blanket `extra="forbid"` on the envelope.** Rules A and B fail loud on the load-bearing
decision (is there a resolvable config), not on every unknown field. A caller can still attach
extra metadata.
- **No OpenAPI.** Keeping the services' OpenAPI off stands. `/inspect` is the live contract. This
plan does not touch that decision.
- **The self-hydration fix is separate and complementary.** Removing the agent's seeded
parameters (`utils.py:285-287` -> `WorkflowRevisionData()`) would make a references-only agent
call self-hydrate like completion and chat. That is a real fix, tracked as a follow-up, and is
NOT bundled here. Validation is the deliverable because it helps even when self-hydration is
intentionally off: a caller who sends a wrong nesting or a bare application still gets a clear
error.

## Consistency across agent / completion / chat

The three services share the resolver, so Rules A and B live in the shared boundary and all three
get the same clear 400 for a wrong nesting or a bare application. The validator inspects only what
the caller sent, not what the resolver would fall back to, so it does not depend on the
agent-specific seeded default. Because there is no "nothing to run" rule, completion and chat keep
their empty-body behavior unchanged.

## Error model

Use the existing `bad_request` helper the family validator already uses (`_raise_bad_request`,
`resolver.py`), which sets `status_code = 400`. The message names which rule failed, shows the
correct shape (Rule A) or explains why the reference is not resolvable (Rule B), and lists the two
valid call shapes via the shared `_INVOKE_CALL_SHAPES` tail.

## Where the code changes landed

- `sdks/python/agenta/sdk/middlewares/running/resolver.py`: `_validate_resolvable_config`
(Rules A and B) plus `_RESOLVABLE_REFERENCE_KEYS` and `_INVOKE_CALL_SHAPES`, sitting next to
`_validate_executable_reference_families` and called at the top of `ResolverMiddleware.__call__`.
One boundary, all three services.
- `sdks/python/oss/tests/pytest/utils/test_resolver_middleware.py`: `TestResolverConfigValidation`.
- Not touched: `sdks/python/agenta/sdk/models/workflows.py` (the nesting check stays in the
resolver) and `services/oss/src/agent/app.py` (the shared message is clear enough).

## Supersedes / merges

This plan supersedes and merges the earlier `harden-invoke` decision and the `silent-fallback` /
`invoke-contract` threads under
`docs/design/agent-workflows/scratch/console/builder-kit/`. Their conclusion (add clear-error
validation, keep OpenAPI off, do not blanket-forbid) is carried forward here and reframed around
request validation and the two valid call shapes.

## Test plan (shipped)

Unit tests in `TestResolverConfigValidation`
(`sdks/python/oss/tests/pytest/utils/test_resolver_middleware.py`):

- References only, bare `application` -> 400 naming variant/environment/revision (Rule B). Also a
bare `workflow` root -> 400.
- References only, a resolvable variant / environment / revision -> passes (no reject).
- Single-nested `data.revision` -> 400 with the nesting fix (Rule A).
- Double-nested `data.revision` -> passes (no reject).
- Inline `data.parameters` -> passes (no reject).
- Empty body -> passes (no "nothing to run" rule; completion / chat do not regress).
- The bare-application reject also fires through `ResolverMiddleware`, before `call_next`.

Capture a live pass with a replay test (see the `agent-replay-test` skill) once deployed.
Loading
Loading