Repository navigation
Conversation
- workflow_engine.providers / secrets / default_provider / default_model with SecretRef, SecretBinding and WorkflowInferenceProvider; one credential per entry; load-time checks (approved executor_type, bound env credential before cloud-agents#269, defaults resolve, no duplicates) - reject allowed_models: [] for workflow and Llama Stack providers - workflow/catalog.py: resolve the run provider through the catalog; the stack rebuilds the provider dict from operator config; credential_ref must match the entry; deprecated credentials_secret accepted only as a logical name with Deprecation/Sunset headers; same-entry rule and executor-type rewrite for definition.provider and step overrides - authz_context records the catalog provider and credential ref name - examples/workflows: cases tagged by phase and run against the real handler by tests/unit/cloud_agents/test_workflow_contract.py; fix wrong template syntax in the example workflows and validate them with cloud-agents' validate_definition Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
beesarmy
left a comment
beesarmy
left a comment
There was a problem hiding this comment.
Reviewed the Phase 1 catalog and registry implementation against the installed cloud-agents executor. Three reproducible P2 findings need fixes before merge; see inline comments.
Validation: 158 focused tests passed, 27 skipped. Direct reproductions confirmed malformed-definition exceptions, discarded base_url, and environment-key normalization mismatches. No e2e suite was run.
| return definition | ||
| normalized = copy.deepcopy(definition) | ||
| scopes = [(normalized, "provider")] | ||
| steps = normalized.get("spec", {}).get("steps", []) |
There was a problem hiding this comment.
[P2] Guard definition shape before walking provider overrides
In governed mode, normalize_definition runs before _validate_workflow_submission, but assumes spec is a dictionary and spec.steps is iterable. RunWorkflowRequest.definition accepts arbitrary dictionaries, so requests with spec: null, spec: [], or spec.steps: null reach this code and raise uncaught AttributeError/TypeError. These become HTTP 500 instead of the existing 422 shape response. The installed cloud-agents validator reports these cases correctly. Check the container types before traversing and let the shape validator return 422; add endpoint regression coverage for these inputs.
| engine: WorkflowEngineConfiguration, entry: WorkflowInferenceProvider, model: str | ||
| ) -> dict[str, str]: | ||
| """Build the provider dict for cloud-agents from operator config only.""" | ||
| provider = {"name": entry.executor_type, "model": model} |
There was a problem hiding this comment.
[P2] Preserve the configured endpoint or reject unsupported endpoint configuration
WorkflowInferenceProvider.base_url is advertised as the operator-set endpoint for Azure and self-hosted models, but _build_provider copies only name, model, and credentials_secret. A configured URL is therefore silently discarded before execution. A provider with base_url: https://team.example/v1 resolves to a dictionary without that field, so execution cannot use the selected catalog endpoint and instead uses ambient/default endpoint configuration or fails. Carry the endpoint through a supported executor contract, or reject non-null base_url at load time until that contract exists. Simply adding base_url to this dictionary is insufficient: the installed inference_spec_from_provider_config rejects it as an unknown field.
| if entry.credential is not None: | ||
| binding = engine.binding(entry.credential.name) | ||
| if binding is not None and binding.env is not None: | ||
| provider["credentials_secret"] = binding.env |
There was a problem hiding this comment.
[P2] Validate env bindings against the executor's actual key lookup
SecretBinding.env accepts any string and is passed unchanged as credentials_secret. The installed executor's resolve_credential_env_key uppercases that reference and replaces hyphens with underscores. A valid case-sensitive binding env: team_key loads successfully, but looks up TEAM_KEY instead of team_key; if only the configured variable is set, no credential resolves, and if both exist, the wrong credential can be selected. Require canonical compatible environment names at configuration load, or implement an exact-name executor handoff. Add a regression that checks actual executor credential resolution rather than only the rebuilt dictionary.
2 participants
Summary
Phase 1 of the #51 plan (catalog and registry). Callers now send logical provider names; the stack maps them to operator config and builds the credential reference itself.
workflow_engine.providers/secrets/default_provider/default_model, withSecretRef,SecretBinding,WorkflowInferenceProvider. One credential per entry.executor_type(checked against the installed cloud-agents), bound credential, env-backed before cloud-agents#269, defaults resolve anddefault_modelset, no duplicates.allowed_models: []is rejected for workflow providers and for Llama Stackinference.providers.workflow/catalog.py: governed mode (non-empty catalog) vs legacy mode (unchanged apart from rejectingcredential_ref). The provider dict handed to cloud-agents is rebuilt from operator config, never from caller keys.credentials_secretis accepted only as the entry's logical credential name, withDeprecation: @1790812800(RFC 9745) andSunset: Thu, 01 Apr 2027 00:00:00 GMT(RFC 8594).definition.providerand stepinference_provider(moved up from Phase 2: logical names must be rewritten to executor types before cloud-agents validates, so the rule belongs with the rewrite). Overrides must name a catalog entry; the caller's definition is never mutated.authz_contextrecords the catalog provider and credential ref name (names only).examples/workflows: every case now carries aphase, andtests/unit/cloud_agents/test_workflow_contract.pyruns them against the real handler and config (38 run, 27 skipped for Phases 2 and 3). Also fixed the example workflows' template syntax, which failed cloud-agents' ownvalidate_definition;verify.pynow checks that.Breaking
credential_refwith 400.credentials_secretis now 400 (was 422): it is rejected at provider resolution.Test plan
uv run make test-unit: 3505 passed, 28 skipped; format, schema, pyright clean on new codeuv run python examples/workflows/verify.pypassesmodeland missingdefault_modelfixed with tests🤖 Generated with Claude Code