feat(agentex): filter GET /agents by agent card metadata - #411
feat(agentex): filter GET /agents by agent card metadata#411declan-scale wants to merge 4 commits into
Conversation
✱ Stainless preview buildsThis PR will update the openapi python typescript Edit this comment to update them. They will appear in their respective SDK's changelogs. ✅ agentex-sdk-openapi studio · code · diff
✅ agentex-sdk-typescript studio · code · diff
✅ agentex-sdk-python studio · code · diff
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
basselatscale
left a comment
basselatscale
left a comment
There was a problem hiding this comment.
The JSONB containment direction is right, and the integration coverage proves the important subset behavior: an agent whose card contains additional metadata still matches {"permits_capable": true}.
A few changes are needed before this is ready:
-
list_agents() now fails when called directly without agent_card_metadata. Its default is a FastAPI Query object, so json.loads(agent_card_metadata) raises TypeError. This is currently failing the two authorization unit tests. Please use the Annotated[..., Query(...)] = None form, or otherwise ensure the Python default is actually None, and keep the direct-call tests passing.
-
In AgentRepository.list, use if agent_card_metadata is not None: rather than a truthiness check. Otherwise an explicitly supplied {} silently bypasses the metadata predicate and includes agents with missing metadata.
-
Please ensure the OpenAPI/SDK contract supports an ergonomic mapping input rather than requiring every caller to manually json.dumps it. The required consumer shape is:
client.agents.list(
agent_card_metadata={"permits_capable": True},
)
If the wire parameter must remain JSON encoded, the generated/client layer should perform that encoding. The current string schema generates string-typed SDK parameters.
Once those are fixed, this server-side capability is sufficient for our immediate goal: discovering AgentCard-published workflow descriptors and removing the generated input-contract bundle.
|
Addressed all three review items in 52b3277:
|
basselatscale
left a comment
basselatscale
left a comment
There was a problem hiding this comment.
Follow-up review — all three original items are addressed ✓
Two new warnings worth confirming before merge:
1. Non-finite JSON values in agent_card_metadata
Python's json.loads() accepts NaN, Infinity, and extreme exponents like 1e1000000 which aren't valid interoperable JSON. These will pass the current isinstance(parsed, dict) check but can fail at the PostgreSQL JSONB binding layer with an uncontrolled 500 instead of a clean 400. Low likelihood in practice, but a defense-in-depth gap. Could be a follow-up — e.g. json.loads(agent_card_metadata, parse_constant=lambda _: None) + a math.isfinite walk, or just parse_float=decimal.Decimal.
2. Stainless SDK Parameter/MissingSchema warning
The Stainless bot reports "Defaulted parameter to type: string because no schema was defined" on all three SDK previews. Can you confirm the generated Python and TypeScript SDK type signatures actually expose agent_card_metadata as a dict/object (not str)? If the preview SDKs are correct the warning is cosmetic, but if they defaulted to str it defeats the intent of the content: application/json spec encoding.
|
Both follow-up warnings addressed in 1a4c251. 1. Non-finite JSON values — fixed.
Nested cases ( 2. Stainless I checked the preview build directly rather than assuming: # stainless-sdks/agentex-sdk-python @ preview/.../agx1-1048-agent-card-metadata-filter
# src/agentex/types/agent_list_params.py
class AgentListParams(TypedDict, total=False):
agent_card_metadata: strSo the generator ignores I've dropped it. - name: agent_card_metadata
in: query
required: false
schema:
anyOf:
- type: string
- type: 'null'
description: 'JSON-encoded object used to filter agents on ...'That clears the warning, makes the two containment filters consistent, and means the generated client surface is reproducible from the spec with no hand-editing. The ergonomics you asked for now live in the SDK's hand-written layer instead of fighting the generator — see the paired SDK PR, which adds from agentex.lib.utils.metadata_filters import encode_metadata_filter
client.agents.list(
agent_card_metadata=encode_metadata_filter({"permits_capable": True}),
)It's one call rather than a bare Tests: 26 passed in |
Adds an optional `agent_card_metadata` query parameter to `GET /agents` that applies an exact JSONB containment (`@>`) filter against `registration_metadata.agent_card.metadata`. Agents whose card is missing or does not contain every requested key/value are excluded; the existing pagination, ordering, task filtering and authorization behavior are preserved. Enables discovery flows where consumers publish opt-in capability flags via the AgentCard and need to enumerate only agents that advertise them.
- Use Annotated[str | None, Query(...)] = None so list_agents() called
directly (outside FastAPI) defaults to None instead of a Query object
- Apply the containment predicate on `is not None` so an explicit {}
filter still requires a card metadata object to be present
- Declare the query parameter with `content: application/json` and an
object schema so SDK generators expose a mapping-typed parameter and
perform the JSON wire encoding themselves
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tent-typed param Two follow-up review items on the agent card metadata filter. Python's json module accepts values that are not interoperable JSON: the bare NaN/Infinity constants, float literals that overflow to infinity (1e1000000), and integers too large for CPython to render. All of them satisfy the isinstance(..., dict) check and only fail later at the JSONB bind parameter, turning caller error into an uncontrolled 500. Parse with parse_constant/parse_float hooks that reject them so every malformed input surfaces as a 400. The parameter was declared with content: application/json in the hope that SDK generators would expose it as a mapping and do the JSON encoding themselves. They do not: the generator reports "no schema was defined" and falls back to type: string, so the generated client parameter is a plain str either way. Drop the content-typed override and declare it the same way the already-shipped GET /tasks?task_metadata= filter is declared -- a nullable string carrying a JSON-encoded object -- which clears the generator warning and keeps the two containment filters consistent.
1a4c251 to
ed1708f
Compare
basselatscale
left a comment
basselatscale
left a comment
There was a problem hiding this comment.
The filtering behavior and validation changes look good, and the local Permits vertical-slice test passed against this exact head.
One production concern remains before we adopt this as a polled workflow registry: the new containment query runs against agents.registration_metadata, but the PR does not add a supporting JSONB index. Every Permits harness replica will periodically page through this filtered endpoint, while agent registrations are comparatively write-light. Without an index, that creates a recurring full scan as the registry grows.
Please add a backward-compatible Alembic migration for a concurrent GIN index matching the current predicate, for example:
CREATE INDEX CONCURRENTLY IF NOT EXISTS ix_agents_registration_metadata_gin
ON agents
USING gin (registration_metadata jsonb_path_ops);Please use Alembic's autocommit support because CREATE INDEX CONCURRENTLY cannot run inside the normal migration transaction, and make the downgrade concurrent as well.
I recommend indexing the full registration_metadata expression used by the current query rather than introducing an expression/partial index in this PR. That keeps the implementation aligned with the existing registration_metadata @> ... predicate and avoids an API or repository-query redesign.
Please also add the normal migration-level verification used by this repository. No Redis caching, registry revision key, or pagination change is requested here; Permits will own its refresh interval, jitter, last-known-good snapshot, and submission provenance.
basselatscale
left a comment
basselatscale
left a comment
There was a problem hiding this comment.
One additional regression case would make the paired SDK worker change safe to rely on.
The SDK follow-up will pass an optional AgentCard through AgentexWorker's existing automatic /agents/register call rather than issuing a second registration. Please add an integration test proving idempotent re-registration of the same agent replaces the stored top-level registration_metadata.agent_card and immediately changes metadata-filter results:
- register the agent with card metadata A;
- verify filter A matches;
- re-register the same agent identity with card metadata B;
- verify filter A no longer matches and filter B does.
The current top-level existing_metadata.update(registration_metadata) implementation appears to provide this behavior already, so this should primarily lock the release-update contract rather than require an API redesign.
Please also document or test the supported way to withdraw a previously published AgentCard during rollback. Omitting registration_metadata currently preserves existing metadata, which can leave a stale discoverable descriptor after reverting to a release that no longer publishes a card. An explicit {"agent_card": null} registration may already provide the required non-breaking clearing behavior; if that is the intended contract, please lock it with a test.
This is separate from the previously requested GIN migration, and does not require registry webhooks or a revision API.
… re-registration contract
Add a concurrent GIN index (jsonb_path_ops) on agents.registration_metadata.
The agent_card_metadata filter applies a JSONB containment predicate on this
column, and discovery clients poll the filtered endpoint. The index keeps
that read path off a sequential scan. The migration uses an autocommit block
for CREATE INDEX CONCURRENTLY on both upgrade and downgrade.
Add two integration tests that lock the registration contract for discovery:
- Re-registration of the same agent replaces the stored agent_card, and the
metadata filter reflects the new card immediately.
- Registration with {"agent_card": null} withdraws a published card, while a
registration that omits registration_metadata preserves it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed both reviews in d801b33. GIN index migration — added. Re-registration contract — locked with tests.
Both behaviors come from the existing top-level |
2 participants
🏆 Brought to you by the Golden Agent (Try it out)
Problem
AgentCardpublishes self-description data throughregistration_metadata.agent_card, butGET /agentshas no way to filter on its contents. Discovery flows that want to enumerate agents opting into a specific protocol capability (e.g. Permits' workflow submission protocol) have no server-side hook and must fetch everything client-side.Linear: AGX1-1048
Change
agent_card_metadataquery parameter toGET /agents. The value is a JSON-encoded object; malformed JSON or non-object payloads return400.AgentsUseCase.list, which reserves the keyagent_card_metadatain the repository filters dict.AgentRepository.listapplies a JSONB@>filter at the top level:registration_metadataisNULL, missingagent_card, or missingagent_card.metadataare naturally excluded, and every requested key/value must be present at the correct nesting level.agents.registration_metadatais alreadyJSONB.task_idjoin, authorization id set, andstatus != DELETEDclause).openapi.yamlregenerated by hand to reflect the new query parameter; the paired SDK PR consumes the same spec.Test coverage added
tests/integration/api/agents/test_agents_api.py:limit/page_number.400.tests/unit/use_cases/test_agents_use_case.pythat seed agents directly via the repository and exercise the use-case-to-repo plumbing against real Postgres (single-key, multi-key, absent-card, and omitted-filter cases). Tests use a per-invocation tag so they are safe against session-scoped container reuse.Test plan (for reviewer, since local yarn/uv installs are skipped per Golden Agent policy)
make test-unit) pass, including the two new use-case tests.make test-integration) pass, including the four new API tests.GET /agents?agent_card_metadata={\"permits_capable\":true}against a dev backend seeded with an agent card and confirm only that agent is returned.openapi.yamlstill matches the FastAPI-generated spec (make gen-openapishould produce no further diff).Out of scope / follow-ups
production_deployment_id.Greptile Summary
The PR adds server-side filtering of agents by metadata published in their agent cards.
agent_card_metadataquery parameter.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant Route as GET /agents participant UseCase as AgentsUseCase participant Repository as AgentRepository participant DB as PostgreSQL Client->>Route: agent_card_metadata JSON Route->>Route: Parse and validate object Route->>UseCase: list(agent_card_metadata) UseCase->>Repository: list(filters) Repository->>DB: "registration_metadata @> nested agent card metadata" DB-->>Repository: Authorized matching agents Repository-->>Client: Paginated agent listReviews (5): Last reviewed commit: "feat(agentex): add a GIN index for the a..." | Re-trigger Greptile
Context used (3)