From 423bca273aad3b7ddcca2564ccd5bf063f697d5f Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Thu, 21 May 2026 15:04:52 -0400 Subject: [PATCH] feat(providers): structured runtime.provider config for Copilot custom endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #136. `runtime.provider` now accepts either the bare string shorthand (`provider: copilot`) or a structured `ProviderSettings` object that forwards a `ProviderConfig` to the Copilot SDK's `create_session(provider=...)` parameter. This lets workflows route the Copilot SDK at OpenAI-compatible / Azure / Anthropic endpoints (Ollama, vLLM, LM Studio, Azure OpenAI, etc.) instead of being locked to the GitHub Copilot service. Schema (`src/conductor/config/schema.py`): - New `ProviderSettings` model (frozen) with `name` / `type` / `wire_api` / `base_url` / `api_key` / `bearer_token` / `headers` / `azure`. `frozen=True` avoids the Pydantic gotcha where cross-field `model_validator` invariants don't re-fire on per-attribute assignment. - `api_key` and `bearer_token` are `SecretStr` (redacted in `model_dump`, dashboard payloads, event logs). - New `AzureProviderOptions` mirrors the SDK's nested `azure.api_version` shape. - `RuntimeConfig.provider` is `ProviderSettings` with a `mode='before'` validator that coerces strings, plus `validate_assignment=True` so CLI mutation still revalidates. - Strict validators reject anchorless / empty / broken combinations that would silently no-op at the SDK boundary: `wire_api` / `type` / `headers` / `azure` cannot stand alone without an endpoint anchor (`base_url` / `api_key` / `bearer_token`); empty `headers`, empty `SecretStr`, and `azure: {api_version: null}` are rejected. Non- copilot `name` plus any structured field is rejected (structured config for Claude / openai-agents is out of scope here). - `model_serializer` collapses `ProviderSettings` back to a bare string when only `name` is set so serialized YAML/JSON stays byte-compatible with the prior schema. Copilot provider (`src/conductor/providers/copilot.py`): - New `provider_settings` kwarg on `__init__`. Tracks `_default_model_explicit` so the custom-routing warning fires based on whether the caller supplied a model, not on a fragile string comparison against the SDK fallback. - `_resolve_sdk_provider_config()` builds the SDK `ProviderConfig` dict with env-var fallbacks: `COPILOT_PROVIDER_BASE_URL` → `OPENAI_BASE_URL` for `base_url`; `COPILOT_PROVIDER_API_KEY` (only) for `api_key`; `COPILOT_PROVIDER_BEARER_TOKEN` (only) for `bearer_token`. Ambient `OPENAI_API_KEY` is intentionally NOT a fallback — that would silently leak an OpenAI credential to whatever `base_url` points at. Activation is gated on `has_custom_routing()` so ambient env vars alone never divert default Copilot traffic. - `_apply_provider_config()` mutates session kwargs in-place; called from both the agent `_execute_sdk_call` path and the dialog `execute_dialog_turn` path so all sessions hit the same endpoint. - Raises `ProviderError` when custom routing activated but every resolved field is falsy (silent no-op was previously possible if all expected env vars were missing). - Warns when `api_key` and `bearer_token` BOTH resolve (from any YAML × env combination) since the SDK silently prefers `bearer_token`; warns when custom routing is active without an explicit `runtime.default_model`. Plumbing: - `providers/factory.py` `create_provider` accepts `provider_settings` and forwards to `CopilotProvider` only. - `providers/registry.py` passes `runtime.provider` to the factory only when the resolved provider type matches the settings' `name`; switches all bare-string reads to `.name`. - `cli/run.py` reads `.name` for logging, adds a redacted `_describe_provider()` helper used in verbose output, and emits a notice when `--provider` override discards structured YAML fields (wired into both `run` and `resume` for parity). Docs & example: - `examples/copilot-local-llm.yaml` — Ollama + Azure OpenAI examples. - `AGENTS.md` — new bullet under "Key Patterns" documenting the object form, env-var fallbacks, and CLI override semantics. Tests: - `tests/test_config/test_provider_settings.py` (new) — coercion, validators (including the new anchorless / empty / broken-combo rejection cases), serialization, `has_custom_routing` gate. - `tests/test_providers/test_copilot_provider_routing.py` (new) — resolver behavior (env precedence, secret unwrap, ambient `OPENAI_API_KEY` non-fallback regression, YAML×env dual-credential warning, raise-on-empty regression, default-model warning suppressed when caller supplies a model) and `create_session` plumbing for both agent and dialog paths plus registry forwarding. - Updated assertion sites across the test tree to read `runtime.provider.name` now that the field is a model. Follow-ups (called out for later): - Structured provider config for `claude` / `openai-agents`. - Persisting a redacted provider fingerprint in checkpoints so resume can warn on configuration drift. - Per-field CLI overrides (e.g. `--provider-base-url`). - Converting `ProviderSettings` to a discriminated union on `name` — would eliminate the cross-field validator in favor of structural per-branch typing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 1 + CHANGELOG.md | 33 ++ README.md | 26 + docs/configuration.md | 131 +++++ examples/README.md | 20 + examples/copilot-local-llm.yaml | 95 ++++ .../conductor/references/yaml-schema.md | 84 ++- src/conductor/cli/run.py | 54 +- src/conductor/config/schema.py | 209 ++++++- src/conductor/providers/copilot.py | 167 +++++- src/conductor/providers/factory.py | 25 +- src/conductor/providers/registry.py | 8 +- tests/test_cli/test_resume_command.py | 2 +- tests/test_cli/test_web_flags.py | 3 +- .../test_backward_compatibility.py | 17 +- tests/test_config/test_loader.py | 6 +- tests/test_config/test_provider_settings.py | 222 ++++++++ tests/test_config/test_schema.py | 11 +- .../test_end_to_end_claude.py | 4 +- .../test_integration/test_mixed_providers.py | 6 +- .../test_copilot_provider_routing.py | 525 ++++++++++++++++++ tests/test_providers/test_registry.py | 1 + 22 files changed, 1616 insertions(+), 34 deletions(-) create mode 100644 examples/copilot-local-llm.yaml create mode 100644 tests/test_config/test_provider_settings.py create mode 100644 tests/test_providers/test_copilot_provider_routing.py diff --git a/AGENTS.md b/AGENTS.md index ae893226..2985e788 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -127,6 +127,7 @@ make validate-examples # validate all examples - **Route evaluation**: First matching `when` condition wins; no `when` = always matches - **Tool resolution**: `null` = all workflow tools, `[]` = none, `[list]` = subset - **Reasoning effort**: `runtime.default_reasoning_effort` sets a workflow-wide default; per-agent `reasoning.effort` overrides it. Allowed values: `low`, `medium`, `high`, `xhigh`. Each provider translates the unified value to its native API (Copilot: `reasoning_effort` on the session, validated against the model's `supported_reasoning_efforts`; Claude: extended thinking with budget mapping low=2048, medium=8192, high=16384, xhigh=32768 tokens, with `temperature` coerced to 1.0 and `max_tokens` bumped to fit the budget). See `examples/reasoning-effort.yaml`. +- **Structured `runtime.provider` (Copilot custom routing)**: `runtime.provider` accepts either the bare string shorthand (`provider: copilot`) or a structured `ProviderSettings` object that routes the Copilot SDK at OpenAI-compatible / Azure / Anthropic endpoints (Ollama, vLLM, LM Studio, Azure OpenAI, etc.). Object fields: `name` (defaults to `copilot`), `type` (`openai`|`azure`|`anthropic`), `wire_api` (`completions`|`responses`), `base_url`, `api_key`, `bearer_token`, `headers`, `azure.api_version`. `api_key` and `bearer_token` are `SecretStr` (redacted in `model_dump` / dashboard / event logs). The model is frozen after construction. Custom routing activates only when at least one non-`name` field is set in YAML — ambient `OPENAI_*` env vars never divert default routing on their own. Once activated, missing fields fall back from env vars in this order: `base_url` ← `COPILOT_PROVIDER_BASE_URL` → `OPENAI_BASE_URL`; `api_key` ← `COPILOT_PROVIDER_API_KEY` (only — ambient `OPENAI_API_KEY` is intentionally NOT a fallback to avoid credential leaks); `bearer_token` ← `COPILOT_PROVIDER_BEARER_TOKEN`. The schema rejects every non-`name` field when `name != "copilot"` (structured config for other providers is a follow-up). It also rejects anchorless / broken combinations that would silently no-op at the SDK boundary: `wire_api` / `type` / `headers` / `azure` cannot stand alone without `base_url` / `api_key` / `bearer_token`; empty `headers`, empty `SecretStr`, and `azure: {api_version: null}` are rejected. The resolver raises `ProviderError` when custom routing is activated but every resolved field is falsy (e.g. expected env vars all unset). Custom routing applies to both agent execution and dialog turns so all sessions hit the same endpoint. `--provider ` CLI override replaces the whole `ProviderSettings` (logs a notice when YAML had structured fields). See `examples/copilot-local-llm.yaml`. ### Debugging `--web-bg` failures diff --git a/CHANGELOG.md b/CHANGELOG.md index 101f89c5..ba918c45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/microsoft/conductor/compare/v0.1.17...HEAD) +### Added +- `runtime.provider` now accepts either the bare string shorthand + (`provider: copilot`) or a structured `ProviderSettings` object that + forwards a `ProviderConfig` to the Copilot SDK's + `create_session(provider=…)` parameter. This lets workflows route the + Copilot SDK at OpenAI-compatible / Azure / Anthropic endpoints — + Ollama, vLLM, LM Studio, Azure OpenAI, llamafile, or any other + OpenAI-compatible REST endpoint — instead of being locked to the + GitHub Copilot service. The structured form supports `name`, `type` + (`openai`|`azure`|`anthropic`), `wire_api` + (`completions`|`responses`), `base_url`, `api_key`, `bearer_token`, + `headers`, and `azure.api_version`. `api_key` and `bearer_token` are + Pydantic `SecretStr` (redacted in `model_dump`, dashboard payloads, + event logs, and checkpoints). Custom routing activates only when YAML + sets at least one non-`name` field — ambient `OPENAI_*` env vars + never divert default routing on their own. Once activated, missing + fields fall back from `COPILOT_PROVIDER_BASE_URL` → `OPENAI_BASE_URL` + for `base_url`, `COPILOT_PROVIDER_API_KEY` for `api_key`, and + `COPILOT_PROVIDER_BEARER_TOKEN` for `bearer_token`. Ambient + `OPENAI_API_KEY` is intentionally NOT consulted as an implicit + fallback (credential-leak risk); use `api_key: ${OPENAI_API_KEY}` + YAML interpolation for explicit opt-in. The schema rejects every + non-`name` field when `name != "copilot"` (structured config for + Claude / openai-agents is a follow-up), and rejects anchorless or + empty combinations (`wire_api` / `type` / `headers` / `azure` alone, + empty `headers`, empty `SecretStr`, empty `azure` block) so silent + no-ops cannot reach the SDK. Custom routing applies to both agent + execution and dialog turns so all sessions hit the same endpoint. + See `examples/copilot-local-llm.yaml` and + [Configuration → Custom Provider Routing](docs/configuration.md#custom-provider-routing-ollama--vllm--azure-openai) + ([#225](https://github.com/microsoft/conductor/pull/225), + [#136](https://github.com/microsoft/conductor/issues/136)). + ## [0.1.17](https://github.com/microsoft/conductor/compare/v0.1.16...v0.1.17) - 2026-05-21 ### Added diff --git a/README.md b/README.md index 6175ffb8..063bdcaf 100644 --- a/README.md +++ b/README.md @@ -230,6 +230,32 @@ Set your API key: `export ANTHROPIC_API_KEY=sk-ant-...` **See also:** [Claude Documentation](docs/providers/claude.md) | [Provider Comparison](docs/providers/comparison.md) | [Migration Guide](docs/providers/migration.md) +### Using a Local / Custom LLM Endpoint (Ollama, vLLM, Azure OpenAI, ...) + +`runtime.provider` also accepts a structured object that routes the +Copilot SDK at any OpenAI-compatible / Azure / Anthropic-shaped endpoint. +Useful for local inference (Ollama, vLLM, LM Studio) and managed +deployments (Azure OpenAI): + +```yaml +workflow: + runtime: + provider: + name: copilot + type: openai # openai | azure | anthropic + wire_api: completions # completions | responses + base_url: http://localhost:11434/v1 + api_key: ${OPENAI_API_KEY:-ollama} + default_model: llama3.1 # match your endpoint's model name +``` + +The structured form is opt-in: a bare `provider: copilot` keeps the +default GitHub Copilot routing. See +[`examples/copilot-local-llm.yaml`](examples/copilot-local-llm.yaml) for +the full example (including an Azure OpenAI variant) and +[Configuration Guide → Custom Provider Routing](docs/configuration.md#custom-provider-routing-ollama--vllm--azure-openai) +for environment-variable fallbacks, security notes, and validator rules. + ## CLI Reference ### `conductor run` diff --git a/docs/configuration.md b/docs/configuration.md index 679469b4..79f82863 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -71,6 +71,137 @@ workflow: **See**: [Claude Provider Documentation](providers/claude.md) +### Custom Provider Routing (Ollama / vLLM / Azure OpenAI) + +`runtime.provider` accepts either the bare string shorthand +(`provider: copilot`) **or** a structured object that forwards a +`ProviderConfig` to the Copilot SDK's `create_session(provider=…)` +parameter. This lets workflows route the Copilot SDK at: + +- Local OpenAI-compatible servers — Ollama, vLLM, LM Studio, llamafile +- Azure OpenAI deployments +- Anthropic-compatible proxies +- Any other OpenAI-compatible REST endpoint + +```yaml +workflow: + runtime: + provider: + name: copilot + type: openai # openai | azure | anthropic + wire_api: completions # completions | responses + base_url: http://localhost:11434/v1 + api_key: ${OPENAI_API_KEY:-ollama} + default_model: llama3.1 # required for non-Copilot endpoints +``` + +Azure OpenAI variant: + +```yaml +workflow: + runtime: + provider: + name: copilot + type: azure + base_url: https://.openai.azure.com + api_key: ${AZURE_OPENAI_API_KEY} + azure: + api_version: "2024-10-21" + default_model: gpt-4o +``` + +#### Activation rule (opt-in) + +Custom routing activates **only** when at least one non-`name` field is +set in YAML. Ambient `OPENAI_*` environment variables alone will NOT +divert default Copilot traffic — that would be too easy a way to break +a workflow based on unrelated shell state. A bare `provider: copilot` +always means default GitHub Copilot routing. + +#### Environment-variable fallbacks + +Once a structured object opts in, missing fields fall back to env vars +in this precedence: + +| Field | Env-var chain | +|---|---| +| `base_url` | `COPILOT_PROVIDER_BASE_URL` → `OPENAI_BASE_URL` | +| `api_key` | `COPILOT_PROVIDER_API_KEY` *(only)* | +| `bearer_token` | `COPILOT_PROVIDER_BEARER_TOKEN` *(only)* | +| `type` | defaults to `"openai"` when `base_url` is set | + +Ambient `OPENAI_API_KEY` is intentionally **not** consulted as an +implicit fallback — that would silently send an OpenAI dev credential +to whatever `base_url` points at, which is a real credential-leak risk. +Users who want OpenAI-environment-style behavior must opt in +explicitly via `api_key: ${OPENAI_API_KEY}` interpolation in YAML. + +#### Secrets + +`api_key` and `bearer_token` are stored as Pydantic `SecretStr` — they +redact in `model_dump`, dashboard payloads, event logs, and +checkpoints. Prefer `${VAR}` env interpolation for the values in YAML +so the literal secret never lands in `workflow_started` events: + +```yaml +api_key: ${OPENAI_API_KEY} # good — interpolated at load time +api_key: sk-aaaaaaaaaaaaaaaa # avoid — literal in yaml_source +``` + +If both `api_key` and `bearer_token` resolve (from any combination of +YAML and env), both are forwarded; the Copilot SDK silently prefers +`bearer_token`, and conductor logs a warning so the precedence is +visible. + +#### Validator rules + +`ProviderSettings` is frozen after construction. The schema rejects +the following misconfigurations at config load time so they cannot +silently produce a no-op SDK call: + +- `name != "copilot"` combined with **any** non-`name` field + (structured config for `claude` / `openai-agents` is not yet + implemented). +- `type: azure` without an `azure: { api_version: ... }` block + (and the reverse: `azure` block without `type: azure`). +- Anchorless routing fields: `wire_api`, `type`, `headers`, or + `azure` cannot stand alone — at least one of `base_url`, `api_key`, + `bearer_token` must also be set (in YAML or via the + `COPILOT_PROVIDER_*` env vars). +- Empty `headers: {}`, empty `api_key: ""`, empty `bearer_token: ""`, + empty `azure: { api_version: null }`. + +When custom routing activates but every resolved field ends up empty +(for example, the workflow expects `COPILOT_PROVIDER_*` env vars and +none are set), the resolver raises `ProviderError` with a clear +message rather than silently routing back to default Copilot. + +#### CLI override + +`--provider ` (and `-p`) replaces the entire `ProviderSettings` +with the bare-string default for that name. When YAML had structured +fields, conductor logs a notice telling the user the custom routing +was dropped: + +``` +Provider override: claude +Provider override discards structured runtime.provider settings (base_url/type/etc.) from YAML; using SDK defaults. +``` + +#### Custom routing and dialog mode + +The resolved provider config is attached to **every** Copilot +`create_session` call this provider makes — including the dialog-mode +turns used by `agent.dialog` evaluators. All sessions hit the same +endpoint, so you can mix custom-routed agents with dialog mode without +worrying about per-call drift. + +#### Example workflow + +[`examples/copilot-local-llm.yaml`](../examples/copilot-local-llm.yaml) +demonstrates the full pattern with both Ollama (active) and Azure +OpenAI (commented variant). + ## Common Configuration Options These options work with both providers: diff --git a/examples/README.md b/examples/README.md index 254a9cc7..a3bc0b9e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -128,6 +128,26 @@ Research workflow demonstrating multi-provider patterns. Demonstrates: conductor run examples/multi-provider-research.yaml --input topic="Cloud computing" ``` +### copilot-local-llm.yaml + +Routes the Copilot SDK at a local / custom OpenAI-compatible endpoint +(Ollama, vLLM, LM Studio, Azure OpenAI, etc.) via structured +`runtime.provider` configuration. Demonstrates: +- Object form of `runtime.provider` (the bare-string `provider: copilot` + shorthand still works) +- `type` / `wire_api` / `base_url` / `api_key` forwarded to the Copilot + SDK's `create_session(provider=…)` parameter +- Secret hygiene via `${OPENAI_API_KEY:-ollama}` interpolation +- Azure OpenAI variant in a commented block + +```bash +# Requires Ollama running on http://localhost:11434 +conductor run examples/copilot-local-llm.yaml --input question="What is Python?" +``` + +See [Configuration → Custom Provider Routing](../docs/configuration.md#custom-provider-routing-ollama--vllm--azure-openai) +for env-var fallbacks, validator rules, and the security rationale. + ## Planning and Implementation ### plan.yaml diff --git a/examples/copilot-local-llm.yaml b/examples/copilot-local-llm.yaml new file mode 100644 index 00000000..667e0838 --- /dev/null +++ b/examples/copilot-local-llm.yaml @@ -0,0 +1,95 @@ +# Copilot SDK pointed at a local / custom LLM endpoint +# +# Demonstrates structured ``runtime.provider`` configuration (issue #136). +# The Copilot SDK's ``create_session(provider=...)`` parameter is exposed +# through conductor's YAML so workflows can target: +# +# - Local OpenAI-compatible servers (Ollama, vLLM, LM Studio, llamafile, ...) +# - Azure OpenAI deployments +# - Any other OpenAI-compatible REST endpoint +# +# Usage (Ollama running on http://localhost:11434): +# +# conductor run examples/copilot-local-llm.yaml --input question="What is Python?" +# +# ``api_key`` uses ``${OPENAI_API_KEY:-ollama}`` YAML interpolation: the +# literal value comes from ``$OPENAI_API_KEY`` when set, otherwise falls +# back to the placeholder ``"ollama"`` (Ollama accepts any non-empty key). +# Either way the literal secret never lands in checkpoints or dashboard +# events because env interpolation happens at load time. +# +# When custom routing is active and a field is omitted from YAML, the +# Copilot provider also consults environment variables: +# +# base_url <- COPILOT_PROVIDER_BASE_URL, then OPENAI_BASE_URL +# api_key <- COPILOT_PROVIDER_API_KEY (NOT ambient OPENAI_API_KEY; +# that would leak a dev creds +# to whatever base_url points +# at — use the ${OPENAI_API_KEY} +# YAML interpolation above for +# explicit opt-in.) +# bearer_token <- COPILOT_PROVIDER_BEARER_TOKEN +# +# The ``runtime.provider`` field still accepts the bare string form +# (``provider: copilot``); the object form below opts into custom routing. + +workflow: + name: copilot-local-llm + description: Route the Copilot SDK at a local OpenAI-compatible endpoint + version: "1.0.0" + entry_point: answerer + + runtime: + # Custom routing: ``name`` plus at least one extra field activates + # the alternate endpoint. Setting only ``name: copilot`` is the + # default GitHub Copilot routing. + provider: + name: copilot + type: openai # openai | azure | anthropic + wire_api: completions # completions | responses + base_url: http://localhost:11434/v1 + api_key: ${OPENAI_API_KEY:-ollama} + + # Custom endpoints rarely expose the SDK's built-in default ``gpt-4o``, + # so always set ``default_model`` to a model your endpoint actually + # serves. The Copilot provider warns when custom routing is active + # without a default model configured. + default_model: llama3.1 + + input: + question: + type: string + required: true + description: The question to answer + +agents: + - name: answerer + description: Answers the user's question via the configured endpoint + prompt: | + You are a helpful assistant. Please answer the following question + clearly and concisely: + + Question: {{ workflow.input.question }} + + Provide a direct answer without unnecessary preamble. + output: + answer: + type: string + description: The answer to the question + routes: + - to: $end + +output: + answer: "{{ answerer.output.answer }}" + +# Azure OpenAI variant (commented out): +# +# runtime: +# provider: +# name: copilot +# type: azure +# base_url: https://.openai.azure.com +# api_key: ${AZURE_OPENAI_API_KEY} +# azure: +# api_version: "2024-10-21" +# default_model: gpt-4o diff --git a/plugins/conductor/skills/conductor/references/yaml-schema.md b/plugins/conductor/skills/conductor/references/yaml-schema.md index a2be3bf2..1a57c817 100644 --- a/plugins/conductor/skills/conductor/references/yaml-schema.md +++ b/plugins/conductor/skills/conductor/references/yaml-schema.md @@ -27,7 +27,8 @@ workflow: # Runtime configuration runtime: - provider: string # "copilot" (default), "claude", or "openai-agents" + provider: string | object # "copilot" (default), "claude", or "openai-agents" + # — or a ProviderSettings object (see below) default_model: string # Default model for all agents temperature: float # 0.0-1.0, controls randomness (optional) max_tokens: integer # Max OUTPUT tokens per response, 1-200000 (optional) @@ -534,6 +535,87 @@ runtime: tools: ["search", "fetch"] # Only these tools (not ["*"]) ``` +## Custom Provider Routing (Ollama / vLLM / Azure OpenAI) + +`runtime.provider` accepts either the bare string shorthand +(`provider: copilot`) or a structured `ProviderSettings` object that +routes the Copilot SDK at OpenAI-compatible / Azure / Anthropic +endpoints (Ollama, vLLM, LM Studio, Azure OpenAI, etc.). + +### Schema + +```yaml +runtime: + provider: + name: string # "copilot" (default), "claude", "openai-agents" + type: string # "openai" | "azure" | "anthropic" (Copilot-only) + wire_api: string # "completions" | "responses" (Copilot-only) + base_url: string # Endpoint base URL + api_key: string # SecretStr; redacted in dumps. Prefer ${OPENAI_API_KEY}. + bearer_token: string # SecretStr; takes precedence over api_key. + headers: {string: string} # Extra HTTP headers (Copilot-only) + azure: # Azure-specific options (requires type: azure) + api_version: string # e.g. "2024-10-21" +``` + +### Local OpenAI-compatible endpoint (Ollama) + +```yaml +runtime: + provider: + name: copilot + type: openai + wire_api: completions + base_url: http://localhost:11434/v1 + api_key: ${OPENAI_API_KEY:-ollama} + default_model: llama3.1 +``` + +### Azure OpenAI + +```yaml +runtime: + provider: + name: copilot + type: azure + base_url: https://.openai.azure.com + api_key: ${AZURE_OPENAI_API_KEY} + azure: + api_version: "2024-10-21" + default_model: gpt-4o +``` + +### Activation and env-var fallbacks + +Custom routing activates **only** when YAML sets at least one +non-`name` field. Ambient env vars alone never divert default +routing. Once activated, missing fields fall back from env: + +| Field | Env-var chain | +|---|---| +| `base_url` | `COPILOT_PROVIDER_BASE_URL` → `OPENAI_BASE_URL` | +| `api_key` | `COPILOT_PROVIDER_API_KEY` *(only)* | +| `bearer_token` | `COPILOT_PROVIDER_BEARER_TOKEN` *(only)* | + +Ambient `OPENAI_API_KEY` is **not** an implicit fallback (would leak +OpenAI credentials to arbitrary `base_url`); use the +`${OPENAI_API_KEY}` YAML interpolation for explicit opt-in. + +### Validator rules + +The schema rejects these misconfigurations at config-load time: + +- `name != "copilot"` with any non-`name` field set +- `type: azure` without `azure: { api_version: ... }` (or vice versa) +- Anchorless fields: `wire_api`, `type`, `headers`, `azure` alone + without `base_url` / `api_key` / `bearer_token` +- Empty containers / `SecretStr`: `headers: {}`, `api_key: ""`, + `bearer_token: ""`, `azure: { api_version: null }` + +When custom routing activates but every resolved field ends up empty, +the Copilot provider raises `ProviderError` rather than silently +falling back to default routing. + ## Validation Rules ### Workflow Validation diff --git a/src/conductor/cli/run.py b/src/conductor/cli/run.py index e626d090..cb5e7117 100644 --- a/src/conductor/cli/run.py +++ b/src/conductor/cli/run.py @@ -28,6 +28,7 @@ from conductor.providers.registry import ProviderRegistry if TYPE_CHECKING: + from conductor.config.schema import ProviderSettings from conductor.events import WorkflowEvent # Verbose console for logging (stderr) @@ -150,6 +151,32 @@ def verbose_log(message: str, style: str = "dim") -> None: _file_console.print(message) +def _describe_provider(provider: ProviderSettings) -> str: + """Render a redacted single-line description of provider settings. + + Used in verbose logs to surface custom routing without leaking + ``api_key`` or ``bearer_token`` values from ``SecretStr`` fields. + """ + if not provider.has_custom_routing(): + return provider.name + parts: list[str] = [provider.name] + if provider.type: + parts.append(f"type={provider.type}") + if provider.wire_api: + parts.append(f"wire_api={provider.wire_api}") + if provider.base_url: + parts.append(f"base_url={provider.base_url}") + if provider.api_key is not None: + parts.append("api_key=***") + if provider.bearer_token is not None: + parts.append("bearer_token=***") + if provider.headers: + parts.append(f"headers={sorted(provider.headers)}") + if provider.azure is not None and provider.azure.api_version: + parts.append(f"azure.api_version={provider.azure.api_version}") + return " ".join(parts) + + def verbose_log_agent_start(agent_name: str, iteration: int) -> None: """Log agent execution start with visual formatting. @@ -1212,9 +1239,20 @@ async def run_workflow_async( if inputs: verbose_log_section("Workflow Inputs", json.dumps(inputs, indent=2)) - # Apply provider override if specified + # Apply provider override if specified. + # Reassigning ``runtime.provider`` to a string re-triggers the + # before-validator on ``RuntimeConfig`` and coerces it back to a + # ``ProviderSettings`` with default fields, intentionally + # discarding any structured custom-routing config from YAML. if provider_override: + had_custom = config.workflow.runtime.provider.has_custom_routing() verbose_log(f"Provider override: {provider_override}", style="yellow") + if had_custom: + verbose_log( + "Provider override discards structured runtime.provider settings " + "(base_url/type/etc.) from YAML; using SDK defaults.", + style="yellow", + ) config.workflow.runtime.provider = provider_override # type: ignore[assignment] # Build workspace instructions preamble @@ -1242,7 +1280,9 @@ async def run_workflow_async( if uses_multi_provider: verbose_log("Multi-provider mode: agents use different providers", style="cyan") else: - verbose_log(f"Single provider mode: {config.workflow.runtime.provider}") + verbose_log( + f"Single provider mode: {_describe_provider(config.workflow.runtime.provider)}" + ) # Use ProviderRegistry for multi-provider support async with ProviderRegistry(config, mcp_servers=mcp_servers) as registry: @@ -1684,9 +1724,17 @@ async def resume_workflow_async( if metadata: config.workflow.metadata.update(metadata) - # Apply provider override if specified (parity with run) + # Apply provider override if specified (parity with run). + # See ``run_workflow_async`` for why we re-validate via assignment. if provider_override: + had_custom = config.workflow.runtime.provider.has_custom_routing() verbose_log(f"Provider override: {provider_override}", style="yellow") + if had_custom: + verbose_log( + "Provider override discards structured runtime.provider settings " + "(base_url/type/etc.) from YAML; using SDK defaults.", + style="yellow", + ) config.workflow.runtime.provider = provider_override # type: ignore[assignment] # Verify the current_agent exists in the workflow diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 43e9d650..675b9444 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -8,7 +8,15 @@ from typing import Any, Literal -from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator +from pydantic import ( + BaseModel, + ConfigDict, + Field, + SecretStr, + field_validator, + model_serializer, + model_validator, +) from conductor.providers.reasoning import ReasoningEffort @@ -821,11 +829,206 @@ def validate_type_requirements(self) -> MCPServerDef: return self +class AzureProviderOptions(BaseModel): + """Azure-specific provider options forwarded to the Copilot SDK. + + Mirrors :class:`copilot.session.AzureProviderOptions`. Currently only + ``api_version`` is recognized; additional fields the SDK adds in the + future can be enumerated here. + """ + + model_config = ConfigDict(extra="forbid") + + api_version: str | None = None + """Azure OpenAI API version (e.g. ``"2024-10-21"``). Optional; the SDK + falls back to its own default when unset.""" + + +class ProviderSettings(BaseModel): + """Structured provider configuration for ``runtime.provider``. + + Supports two YAML shapes via :meth:`RuntimeConfig._coerce_provider`: + + - String shorthand: ``provider: copilot`` (equivalent to + ``provider: {name: copilot}``). + - Object form: enables routing the Copilot SDK at custom endpoints + such as Azure OpenAI, Ollama, vLLM, LM Studio, or any other + OpenAI-compatible server. Object fields beyond ``name`` are + currently supported only for ``name: copilot``; they are forwarded + verbatim to ``copilot.client.create_session(provider=...)``. + + When any field beyond ``name`` is set, the Copilot provider activates + "custom routing" mode and fills any missing field from environment + variables (see :meth:`has_custom_routing`). + + The model is frozen after construction (``frozen=True``) because + custom routing is set-once at config load. This avoids the + Pydantic gotcha where ``model_validator(mode="after")`` + cross-field invariants do not re-fire on per-attribute assignment + even with ``validate_assignment=True``. + """ + + model_config = ConfigDict(extra="forbid", frozen=True) + + name: Literal["copilot", "openai-agents", "claude"] = "copilot" + """SDK provider to use for agent execution.""" + + type: Literal["openai", "azure", "anthropic"] | None = None + """Wire-format dialect for the upstream endpoint. Copilot-only. + + Defaults to ``"openai"`` at activation time when ``base_url`` is set + but ``type`` is not. + """ + + wire_api: Literal["completions", "responses"] | None = None + """OpenAI wire API variant. Copilot-only. + + ``"completions"`` for the classic ``/v1/chat/completions`` shape used by + Ollama, vLLM, LM Studio, and the legacy OpenAI API. ``"responses"`` for + the newer OpenAI Responses API. + """ + + base_url: str | None = None + """Endpoint base URL (e.g. ``http://localhost:11434/v1``).""" + + api_key: SecretStr | None = None + """API key for the endpoint. Prefer ``${OPENAI_API_KEY}`` interpolation + in YAML so the literal value never lands in ``workflow_started`` events + or checkpoints.""" + + bearer_token: SecretStr | None = None + """Bearer token. Takes precedence over ``api_key`` when both are set. + Copilot-only.""" + + headers: dict[str, str] | None = None + """Extra HTTP headers to send with every request. Copilot-only.""" + + azure: AzureProviderOptions | None = None + """Azure-specific options (e.g. ``api_version``). Requires + ``type: azure``. Copilot-only.""" + + @model_validator(mode="after") + def _check_field_compatibility(self) -> ProviderSettings: + copilot_only_fields = { + "type": self.type, + "wire_api": self.wire_api, + "bearer_token": self.bearer_token, + "headers": self.headers, + "azure": self.azure, + } + if self.name != "copilot": + extras = sorted(k for k, v in copilot_only_fields.items() if v is not None) + if extras: + raise ValueError( + f"Provider fields {extras} are only supported when name='copilot'. " + "Structured provider config for other providers is not yet implemented." + ) + if self.base_url is not None or self.api_key is not None: + raise ValueError( + f"Structured provider config (base_url/api_key) for name='{self.name}' " + "is not yet implemented; use environment variables for the underlying SDK." + ) + + if self.azure is not None and self.type != "azure": + raise ValueError("'azure' options require type='azure'") + + # Reject empty containers and empty SecretStr — they activate + # custom routing via has_custom_routing() but resolve to falsy + # values in the resolver and would silently drop the entire + # SDK provider kwarg. + if self.headers is not None and len(self.headers) == 0: + raise ValueError( + "'headers' must contain at least one entry; remove the key to omit headers" + ) + for secret_field, value in (("api_key", self.api_key), ("bearer_token", self.bearer_token)): + if value is not None and value.get_secret_value() == "": + raise ValueError( + f"'{secret_field}' is empty; remove the key or supply a value " + "(typo / unset env interpolation?)" + ) + + # Positive precondition: structured fields that only make sense + # alongside an endpoint must not be the *only* thing set. + # ``base_url`` may still come from an env-var fallback, so this + # check is intentionally narrow: ``wire_api`` / ``type`` / + # ``headers`` / ``azure`` alone (with no other field) is almost + # certainly a misconfiguration. + if self.base_url is None and self.api_key is None and self.bearer_token is None: + anchorless = sorted( + k + for k in ("type", "wire_api", "headers", "azure") + if copilot_only_fields.get(k) is not None + ) + if anchorless: + raise ValueError( + f"Provider fields {anchorless} require base_url, api_key, or " + "bearer_token to also be set (in YAML or via environment variables); " + "they cannot stand alone." + ) + + if self.azure is not None and self.azure.api_version is None: + raise ValueError( + "'azure' block is empty; either set azure.api_version or remove the block" + ) + + return self + + def has_custom_routing(self) -> bool: + """Return True when YAML explicitly opted into custom routing. + + Custom routing is gated on at least one non-``name`` field being + set. We never activate from ambient environment variables alone — + that would silently divert default Copilot traffic based on + unrelated shell state. + """ + return any( + value is not None + for value in ( + self.type, + self.wire_api, + self.base_url, + self.api_key, + self.bearer_token, + self.headers, + self.azure, + ) + ) + + @model_serializer(mode="wrap") + def _serialize(self, nxt: Any) -> Any: + """Collapse to bare string when only ``name`` is set. + + Preserves backward compatibility with the original + ``provider: copilot`` YAML/JSON shape: a ``ProviderSettings`` with + no custom routing round-trips as the plain string ``"copilot"``, + not as ``{"name": "copilot"}``. Once any structured field is set, + the full object is emitted. + """ + if not self.has_custom_routing(): + return self.name + return nxt(self) + + class RuntimeConfig(BaseModel): """Provider and runtime configuration.""" - provider: Literal["copilot", "openai-agents", "claude"] = "copilot" - """SDK provider to use for agent execution.""" + model_config = ConfigDict(validate_assignment=True) + + provider: ProviderSettings = Field(default_factory=ProviderSettings) + """SDK provider configuration. + + Accepts either a string shorthand (``provider: copilot``) or a + structured :class:`ProviderSettings` object. See + :class:`ProviderSettings` for the full field reference and custom + routing semantics. + """ + + @field_validator("provider", mode="before") + @classmethod + def _coerce_provider(cls, value: Any) -> Any: + if isinstance(value, str): + return {"name": value} + return value default_model: str | None = None """Default model for agents that don't specify one.""" diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index 500f67f8..746c4b3e 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -26,7 +26,7 @@ from conductor.providers.reasoning import ReasoningEffort, resolve_reasoning_effort if TYPE_CHECKING: - from conductor.config.schema import AgentDef, OutputField + from conductor.config.schema import AgentDef, OutputField, ProviderSettings logger = logging.getLogger(__name__) @@ -163,6 +163,7 @@ def __init__( temperature: float | None = None, max_agent_iterations: int | None = None, default_reasoning_effort: ReasoningEffort | None = None, + provider_settings: ProviderSettings | None = None, ) -> None: """Initialize the Copilot provider. @@ -184,12 +185,27 @@ def __init__( applied to ``create_session`` when an agent does not specify its own ``reasoning.effort``. One of ``low``, ``medium``, ``high``, ``xhigh``, or ``None`` to send no value. + provider_settings: Optional structured provider settings from + ``runtime.provider``. When ``has_custom_routing()`` is True, + the resolved SDK ``ProviderConfig`` is attached to every + ``create_session`` call (both agent execution and dialog + turns), enabling custom OpenAI-compatible / Azure / Anthropic + endpoints. Env-var fallbacks + (``COPILOT_PROVIDER_BASE_URL`` → ``OPENAI_BASE_URL``, + ``COPILOT_PROVIDER_API_KEY`` → ``OPENAI_API_KEY``, + ``COPILOT_PROVIDER_BEARER_TOKEN``) fill missing fields once + custom routing is activated; ambient OpenAI env vars never + activate custom routing on their own. """ self._client: Any = None # Will hold Copilot SDK client self._mock_handler = mock_handler self._call_history: list[dict[str, Any]] = [] self._retry_config = retry_config or RetryConfig() self._retry_history: list[dict[str, Any]] = [] # For testing retries + # Track whether the caller actually supplied a default model so the + # custom-routing warning can fire reliably without depending on the + # value of the sentinel default below. + self._default_model_explicit = model is not None self._default_model = model or "gpt-4o" self._mcp_servers = mcp_servers or {} self._started = False @@ -203,6 +219,8 @@ def __init__( self._resume_session_ids: dict[str, str] = {} self._interrupted_session: Any = None self._abort_supported: bool | None = None + self._provider_settings = provider_settings + self._warn_custom_routing_default_model() @staticmethod def _default_permission_handler( @@ -220,6 +238,145 @@ def _default_permission_handler( logger.debug("auto-approved permission request: %s", request) return PermissionHandler.approve_all(request, invocation) + def _warn_custom_routing_default_model(self) -> None: + """Warn if custom routing is active but no default model is set. + + Custom endpoints (Ollama, vLLM, Azure deployments) rarely expose + the SDK's built-in default model. Surface this early so users + don't get a confusing 404 on the first ``create_session``. + + Uses ``_default_model_explicit`` (captured in ``__init__``) so + the heuristic does not depend on the value of the sentinel + fallback — a future change to the fallback model name would not + break this warning, and a user who deliberately picks the same + model name as the fallback does not get a false positive. + """ + settings = self._provider_settings + if settings is None or not settings.has_custom_routing(): + return + if not self._default_model_explicit: + logger.warning( + "Custom Copilot provider routing is active (base_url=%s) but no " + "runtime.default_model is set. The SDK fallback %r is unlikely " + "to exist on the custom endpoint; configure runtime.default_model.", + settings.base_url or "", + self._default_model, + ) + + def _resolve_sdk_provider_config(self) -> dict[str, Any] | None: + """Build the SDK ``ProviderConfig`` dict to forward, or ``None``. + + Returns ``None`` when no structured ``runtime.provider`` was + configured, or when the configured object did not activate custom + routing (i.e. only ``name`` was set). When custom routing is + active, fills missing fields from environment variables in this + precedence order: + + - ``base_url``: ``COPILOT_PROVIDER_BASE_URL`` → ``OPENAI_BASE_URL`` + - ``api_key``: ``COPILOT_PROVIDER_API_KEY`` (only) + - ``bearer_token``: ``COPILOT_PROVIDER_BEARER_TOKEN`` (only) + + Ambient ``OPENAI_API_KEY`` is intentionally NOT used as an + implicit fallback for ``api_key`` — that would silently send an + OpenAI credential to whatever ``base_url`` points at, which is + a real credential-leak risk in dev shells. Users who want + OpenAI-environment-style behavior should opt in explicitly via + ``api_key: ${OPENAI_API_KEY}`` interpolation in YAML. + + ``type`` defaults to ``"openai"`` when ``base_url`` is set but + ``type`` is not — OpenAI-compatible endpoints (Ollama, vLLM, LM + Studio) are the dominant use case. + + When both ``api_key`` and ``bearer_token`` resolve (from any + combination of YAML and env vars), both are forwarded; the + Copilot SDK silently prefers ``bearer_token`` and a warning is + emitted. + + Raises ``ProviderError`` when custom routing is activated but + every routing field resolves to a falsy value (e.g. all + intended env vars are unset). Silently returning ``None`` in + that case would mask user misconfiguration as default behavior. + """ + settings = self._provider_settings + if settings is None or not settings.has_custom_routing(): + return None + + base_url = ( + settings.base_url + or os.environ.get("COPILOT_PROVIDER_BASE_URL") + or os.environ.get("OPENAI_BASE_URL") + ) + api_key = ( + settings.api_key.get_secret_value() + if settings.api_key is not None + else os.environ.get("COPILOT_PROVIDER_API_KEY") + ) + bearer_token = ( + settings.bearer_token.get_secret_value() + if settings.bearer_token is not None + else os.environ.get("COPILOT_PROVIDER_BEARER_TOKEN") + ) + + # Operate on the resolved locals so the warning also fires when + # one credential comes from YAML and the other from an env var. + if api_key and bearer_token: + logger.warning( + "Both api_key and bearer_token resolved (possibly from different sources, " + "YAML and/or env vars); the Copilot SDK silently prefers bearer_token." + ) + + provider_type: str | None = settings.type + if provider_type is None and base_url: + provider_type = "openai" + + cfg: dict[str, Any] = {} + if provider_type: + cfg["type"] = provider_type + if settings.wire_api: + cfg["wire_api"] = settings.wire_api + if base_url: + cfg["base_url"] = base_url + if api_key: + cfg["api_key"] = api_key + if bearer_token: + cfg["bearer_token"] = bearer_token + if settings.headers: + cfg["headers"] = dict(settings.headers) + # Always emit the azure block when the settings carry one — the + # schema validator already requires at least ``api_version`` to + # be set, so this never produces an empty sub-dict. + if settings.azure is not None: + azure_cfg: dict[str, Any] = {} + if settings.azure.api_version is not None: + azure_cfg["api_version"] = settings.azure.api_version + cfg["azure"] = azure_cfg + + if not cfg: + raise ProviderError( + "runtime.provider opted into custom routing but no usable fields " + "resolved. Set base_url / api_key / bearer_token in YAML or via the " + "COPILOT_PROVIDER_* environment variables.", + suggestion=( + "Check that any ${VAR} interpolations in runtime.provider resolved " + "to non-empty values and that the intended COPILOT_PROVIDER_* env " + "vars are exported in the current shell." + ), + is_retryable=False, + ) + + return cfg + + def _apply_provider_config(self, session_kwargs: dict[str, Any]) -> None: + """Attach the resolved SDK provider config to ``session_kwargs``. + + Called from every ``create_session`` site (main agent execution and + dialog turns) so all sessions for this provider instance hit the + same endpoint. + """ + provider_cfg = self._resolve_sdk_provider_config() + if provider_cfg is not None: + session_kwargs["provider"] = provider_cfg + async def execute( self, agent: AgentDef, @@ -565,6 +722,10 @@ async def _execute_sdk_call( if self._mcp_servers: session_kwargs["mcp_servers"] = self._mcp_servers + # Apply custom provider routing (Ollama / vLLM / Azure / etc.) + # when runtime.provider opted into it. + self._apply_provider_config(session_kwargs) + # Resolve reasoning effort: per-agent override wins over runtime default. # When set, validate against the model's advertised capabilities # before forwarding to the SDK. @@ -1922,6 +2083,10 @@ async def execute_dialog_turn( "system_message": {"mode": "replace", "content": system_prompt}, } + # Honor the same custom provider routing as agent sessions so + # dialog turns hit the same endpoint as agent execution. + self._apply_provider_config(dialog_kwargs) + # Dialog turns honor the workflow-wide default reasoning effort # only — there's no agent-scoped override at this layer. effort = self._default_reasoning_effort diff --git a/src/conductor/providers/factory.py b/src/conductor/providers/factory.py index 09f504e2..17833b6d 100644 --- a/src/conductor/providers/factory.py +++ b/src/conductor/providers/factory.py @@ -6,7 +6,7 @@ from __future__ import annotations -from typing import Any, Literal +from typing import TYPE_CHECKING, Any, Literal from conductor.exceptions import ProviderError from conductor.providers.base import AgentProvider @@ -14,6 +14,9 @@ from conductor.providers.copilot import CopilotProvider, IdleRecoveryConfig from conductor.providers.reasoning import ReasoningEffort +if TYPE_CHECKING: + from conductor.config.schema import ProviderSettings + async def create_provider( provider_type: Literal["copilot", "openai-agents", "claude"] = "copilot", @@ -26,6 +29,7 @@ async def create_provider( max_session_seconds: float | None = None, max_agent_iterations: int | None = None, default_reasoning_effort: ReasoningEffort | None = None, + provider_settings: ProviderSettings | None = None, ) -> AgentProvider: """Factory function to create the appropriate provider. @@ -49,6 +53,10 @@ async def create_provider( default_reasoning_effort: Workflow-wide default reasoning effort (``low`` / ``medium`` / ``high`` / ``xhigh``) applied when an agent does not specify its own ``reasoning.effort``. + provider_settings: Structured ``runtime.provider`` settings. Only + applied when ``provider_type == "copilot"`` and the settings + opted into custom routing; ignored for all other providers + (structured config for those providers is not yet implemented). Returns: Configured AgentProvider instance. @@ -75,6 +83,7 @@ async def create_provider( idle_recovery_config=idle_recovery_config, max_agent_iterations=max_agent_iterations, default_reasoning_effort=default_reasoning_effort, + provider_settings=provider_settings, ) case "openai-agents": raise ProviderError( @@ -140,7 +149,18 @@ async def create_provider( Raises: ProviderError: If provider creation or validation fails. """ - provider_type = getattr(runtime_config, "provider", "copilot") + provider_settings = getattr(runtime_config, "provider", None) + # Support both the new ProviderSettings object and any legacy + # string-typed mock that test code might still pass in. + if hasattr(provider_settings, "name"): + provider_type = provider_settings.name + elif isinstance(provider_settings, str): + provider_type = provider_settings + provider_settings = None + else: + provider_type = "copilot" + provider_settings = None + default_model = getattr(runtime_config, "model", None) temperature = getattr(runtime_config, "temperature", None) max_tokens = getattr(runtime_config, "max_tokens", None) @@ -159,4 +179,5 @@ async def create_provider( max_session_seconds=max_session_seconds, max_agent_iterations=max_agent_iterations, default_reasoning_effort=default_reasoning_effort, + provider_settings=provider_settings, ) diff --git a/src/conductor/providers/registry.py b/src/conductor/providers/registry.py index 02362002..762b5e54 100644 --- a/src/conductor/providers/registry.py +++ b/src/conductor/providers/registry.py @@ -51,7 +51,7 @@ def __init__( self._config = config self._mcp_servers = mcp_servers self._providers: dict[ProviderType, AgentProvider] = {} - self._default_provider_type: ProviderType = config.workflow.runtime.provider + self._default_provider_type: ProviderType = config.workflow.runtime.provider.name self._resume_session_ids: dict[str, str] = {} @property @@ -108,6 +108,11 @@ async def _get_or_create_provider(self, provider_type: ProviderType) -> AgentPro # Create the provider with runtime config runtime = self._config.workflow.runtime + # Only forward structured provider settings to the matching + # provider — settings for Copilot must not bleed into a per-agent + # Claude override (and vice versa once Claude grows its own + # structured config). + provider_settings = runtime.provider if runtime.provider.name == provider_type else None provider = await create_provider( provider_type=provider_type, validate=True, @@ -118,6 +123,7 @@ async def _get_or_create_provider(self, provider_type: ProviderType) -> AgentPro timeout=runtime.timeout, max_session_seconds=runtime.max_session_seconds, max_agent_iterations=runtime.max_agent_iterations, + provider_settings=provider_settings, ) # Pass stored resume session IDs to newly created providers diff --git a/tests/test_cli/test_resume_command.py b/tests/test_cli/test_resume_command.py index ac73fcfa..44253042 100644 --- a/tests/test_cli/test_resume_command.py +++ b/tests/test_cli/test_resume_command.py @@ -1054,7 +1054,7 @@ def _capture_config(config: Any, **_kwargs: Any) -> Any: # noqa: ANN401 assert captured_configs, "ProviderRegistry was not constructed" cfg = captured_configs[0] - assert cfg.workflow.runtime.provider == "claude" + assert cfg.workflow.runtime.provider.name == "claude" @pytest.mark.asyncio async def test_metadata_merges_into_config(self, tmp_path: Path) -> None: diff --git a/tests/test_cli/test_web_flags.py b/tests/test_cli/test_web_flags.py index d98922ef..e8196f78 100644 --- a/tests/test_cli/test_web_flags.py +++ b/tests/test_cli/test_web_flags.py @@ -16,6 +16,7 @@ from typer.testing import CliRunner from conductor.cli.app import app +from conductor.config.schema import ProviderSettings runner = CliRunner() @@ -252,7 +253,7 @@ async def test_dashboard_start_oserror_is_non_fatal(self) -> None: mock_config.workflow.name = "test" mock_config.workflow.entry_point = "agent1" mock_config.agents = [] - mock_config.workflow.runtime.provider = "copilot" + mock_config.workflow.runtime.provider = ProviderSettings(name="copilot") mock_config.workflow.limits.max_iterations = 50 mock_config.workflow.limits.timeout_seconds = None mock_config.workflow.cost.show_summary = False diff --git a/tests/test_config/test_backward_compatibility.py b/tests/test_config/test_backward_compatibility.py index b120d1a1..56ec5b60 100644 --- a/tests/test_config/test_backward_compatibility.py +++ b/tests/test_config/test_backward_compatibility.py @@ -86,8 +86,8 @@ def test_load_existing_copilot_workflows(self, example_file: Path): assert isinstance(config.workflow.runtime, RuntimeConfig) # Verify provider is copilot (or not explicitly set to claude) - if config.workflow.runtime.provider: - assert config.workflow.runtime.provider in ["copilot", "openai-agents"] + if config.workflow.runtime.provider.name: + assert config.workflow.runtime.provider.name in ["copilot", "openai-agents"] def test_copilot_workflow_with_new_schema_no_validation_errors(self): """Test that Copilot workflows load without validation errors with new schema. @@ -122,7 +122,7 @@ def test_copilot_workflow_with_new_schema_no_validation_errors(self): # Verify basic structure assert config.workflow.name == "test-copilot" - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" assert len(config.agents) == 1 # Verify no validation errors occurred (if we got here, validation passed) @@ -156,7 +156,8 @@ def test_serialization_round_trip_preserves_behavior(self): # Verify runtime config is preserved assert ( - reloaded_config.workflow.runtime.provider == original_config.workflow.runtime.provider + reloaded_config.workflow.runtime.provider.name + == original_config.workflow.runtime.provider.name ) # Verify serialized dict does not contain optional fields when not set @@ -278,7 +279,7 @@ def test_all_examples_load_successfully(self, example_file: Path): assert config.workflow.runtime is not None # If it's a Claude workflow, verify common fields can be accessed - if config.workflow.runtime.provider == "claude": + if config.workflow.runtime.provider.name == "claude": # These fields should be accessible (no assertion on values) _ = config.workflow.runtime.temperature _ = config.workflow.runtime.max_tokens @@ -357,7 +358,7 @@ def test_claude_workflow_schema_validation(self): ) # Verify schema validation passed and fields are set - assert config.workflow.runtime.provider == "claude" + assert config.workflow.runtime.provider.name == "claude" assert config.workflow.runtime.temperature == 0.7 assert config.workflow.runtime.max_tokens == 1024 assert config.workflow.runtime.timeout == 120.0 @@ -403,7 +404,7 @@ def test_missing_provider_defaults_to_copilot(self): config = WorkflowConfig.model_validate(config_dict) # Verify provider defaults to 'copilot' - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" # Verify no optional fields are set assert config.workflow.runtime.temperature is None @@ -443,7 +444,7 @@ def test_explicit_runtime_without_provider_defaults_to_copilot(self): config = WorkflowConfig.model_validate(config_dict) # Verify provider defaults to 'copilot' - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" # Verify serialization doesn't include None optional fields serialized = config.model_dump(mode="json", exclude_none=True) diff --git a/tests/test_config/test_loader.py b/tests/test_config/test_loader.py index 72472556..74130c95 100644 --- a/tests/test_config/test_loader.py +++ b/tests/test_config/test_loader.py @@ -101,7 +101,7 @@ def test_load_valid_full_workflow(self, fixtures_dir: Path) -> None: assert config.workflow.name == "full-workflow" assert config.workflow.version == "1.0.0" - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" assert config.workflow.limits.max_iterations == 20 assert config.workflow.limits.timeout_seconds == 300 assert config.workflow.context.mode == "accumulate" @@ -162,7 +162,7 @@ def test_load_env_vars_resolved(self, fixtures_dir: Path) -> None: loader = ConfigLoader() config = loader.load(fixtures_dir / "valid_env_vars.yaml") - assert config.workflow.runtime.provider == "openai-agents" + assert config.workflow.runtime.provider.name == "openai-agents" assert config.workflow.runtime.default_model == "gpt-4-turbo" assert config.agents[0].model == "gpt-3.5" assert "secret123" in config.agents[0].prompt @@ -243,7 +243,7 @@ def test_load_workflow_with_defaults(self) -> None: config = load_config_string(yaml_content) # Check defaults - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" assert config.workflow.limits.max_iterations == 10 assert config.workflow.limits.timeout_seconds is None # Unlimited by default assert config.workflow.context.mode == "accumulate" diff --git a/tests/test_config/test_provider_settings.py b/tests/test_config/test_provider_settings.py new file mode 100644 index 00000000..a05095bb --- /dev/null +++ b/tests/test_config/test_provider_settings.py @@ -0,0 +1,222 @@ +"""Tests for ``ProviderSettings`` and structured ``runtime.provider`` config. + +Covers issue #136: structured provider configuration that lets the Copilot +SDK be pointed at OpenAI-compatible / Azure / Anthropic endpoints (Ollama, +vLLM, LM Studio, Azure OpenAI, etc.). +""" + +from __future__ import annotations + +import pytest +from pydantic import SecretStr, ValidationError + +from conductor.config.schema import ( + AzureProviderOptions, + ProviderSettings, + RuntimeConfig, +) + + +class TestProviderSettingsCoercion: + """``runtime.provider`` accepts both string shorthand and object form.""" + + def test_string_shorthand_coerces_to_provider_settings(self) -> None: + rc = RuntimeConfig.model_validate({"provider": "copilot"}) + assert isinstance(rc.provider, ProviderSettings) + assert rc.provider.name == "copilot" + assert not rc.provider.has_custom_routing() + + def test_object_form(self) -> None: + rc = RuntimeConfig.model_validate( + { + "provider": { + "name": "copilot", + "type": "openai", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-xxx", + "wire_api": "completions", + } + } + ) + assert rc.provider.name == "copilot" + assert rc.provider.type == "openai" + assert rc.provider.wire_api == "completions" + assert rc.provider.base_url == "http://localhost:11434/v1" + assert isinstance(rc.provider.api_key, SecretStr) + assert rc.provider.api_key.get_secret_value() == "sk-xxx" + assert rc.provider.has_custom_routing() + + def test_default_runtime_has_default_provider(self) -> None: + rc = RuntimeConfig() + assert rc.provider.name == "copilot" + assert not rc.provider.has_custom_routing() + + def test_reassignment_with_string_is_validated(self) -> None: + """``validate_assignment=True`` makes string reassignment work.""" + rc = RuntimeConfig.model_validate( + {"provider": {"name": "copilot", "base_url": "http://x/v1"}} + ) + assert rc.provider.has_custom_routing() + rc.provider = "copilot" # type: ignore[assignment] + assert isinstance(rc.provider, ProviderSettings) + assert rc.provider.name == "copilot" + assert not rc.provider.has_custom_routing(), ( + "string reassignment must reset structured fields" + ) + + +class TestProviderSettingsValidation: + """Cross-field validators reject incompatible combinations.""" + + def test_non_copilot_with_copilot_only_field_rejected(self) -> None: + with pytest.raises(ValidationError, match="only supported when name='copilot'"): + ProviderSettings(name="claude", type="anthropic") + + def test_non_copilot_with_base_url_rejected(self) -> None: + with pytest.raises(ValidationError, match="not yet implemented"): + ProviderSettings(name="claude", base_url="http://anthropic-proxy/v1") + + def test_azure_options_require_azure_type(self) -> None: + with pytest.raises(ValidationError, match="require type='azure'"): + ProviderSettings( + name="copilot", + type="openai", + azure=AzureProviderOptions(api_version="2024-10-21"), + ) + + def test_azure_with_azure_type_accepted(self) -> None: + s = ProviderSettings( + name="copilot", + type="azure", + base_url="https://x.openai.azure.com", + azure=AzureProviderOptions(api_version="2024-10-21"), + ) + assert s.azure is not None + assert s.azure.api_version == "2024-10-21" + + def test_invalid_type_literal_rejected(self) -> None: + with pytest.raises(ValidationError): + ProviderSettings(name="copilot", type="ollama") # type: ignore[arg-type] + + def test_invalid_wire_api_literal_rejected(self) -> None: + with pytest.raises(ValidationError): + ProviderSettings(name="copilot", wire_api="grpc") # type: ignore[arg-type] + + def test_invalid_name_rejected(self) -> None: + with pytest.raises(ValidationError): + ProviderSettings(name="ollama") # type: ignore[arg-type] + + def test_anchorless_type_rejected(self) -> None: + """``type`` alone (no endpoint anchor) is rejected — it cannot + produce a usable SDK provider config.""" + with pytest.raises(ValidationError, match="cannot stand alone"): + ProviderSettings(name="copilot", type="openai") + + def test_anchorless_wire_api_rejected(self) -> None: + with pytest.raises(ValidationError, match="cannot stand alone"): + ProviderSettings(name="copilot", wire_api="completions") + + def test_anchorless_headers_rejected(self) -> None: + with pytest.raises(ValidationError, match="cannot stand alone"): + ProviderSettings(name="copilot", headers={"X-Foo": "1"}) + + def test_empty_headers_rejected(self) -> None: + with pytest.raises(ValidationError, match="at least one entry"): + ProviderSettings(name="copilot", base_url="http://x/v1", headers={}) + + def test_empty_api_key_rejected(self) -> None: + """Empty SecretStr would activate custom routing but resolve to + falsy in the resolver — fail loudly at config time instead.""" + with pytest.raises(ValidationError, match="'api_key' is empty"): + ProviderSettings(name="copilot", base_url="http://x/v1", api_key="") + + def test_empty_bearer_token_rejected(self) -> None: + with pytest.raises(ValidationError, match="'bearer_token' is empty"): + ProviderSettings(name="copilot", base_url="http://x/v1", bearer_token="") + + def test_empty_azure_block_rejected(self) -> None: + """``azure: {}`` (or ``azure.api_version: null``) activates custom + routing but would be silently dropped at the SDK boundary.""" + with pytest.raises(ValidationError, match="'azure' block is empty"): + ProviderSettings( + name="copilot", + type="azure", + base_url="https://x.openai.azure.com", + azure=AzureProviderOptions(), + ) + + +class TestProviderSettingsSerialization: + """Round-trip serialization preserves backward compatibility.""" + + def test_default_serializes_as_bare_string(self) -> None: + """``provider: copilot`` must round-trip as a bare string, not + ``{name: copilot}``, so existing tooling that reads serialized + workflow configs keeps working.""" + rc = RuntimeConfig() + dumped = rc.model_dump(mode="json", exclude_none=True) + assert dumped["provider"] == "copilot" + + def test_custom_routing_serializes_as_object(self) -> None: + rc = RuntimeConfig.model_validate( + {"provider": {"name": "copilot", "base_url": "http://x/v1", "type": "openai"}} + ) + dumped = rc.model_dump(mode="json", exclude_none=True) + assert dumped["provider"] == { + "name": "copilot", + "type": "openai", + "base_url": "http://x/v1", + } + + def test_secrets_redacted_in_dump(self) -> None: + """``SecretStr`` fields must redact in ``model_dump`` (no secrets + in event logs / checkpoints / dashboard payloads).""" + rc = RuntimeConfig.model_validate( + {"provider": {"name": "copilot", "base_url": "http://x/v1", "api_key": "sk-shh"}} + ) + dumped = rc.model_dump(mode="json", exclude_none=True) + # Pydantic SecretStr renders as "**********" in model_dump + assert dumped["provider"]["api_key"] == "**********" + + +class TestHasCustomRouting: + """``has_custom_routing()`` gates env-var fallback activation.""" + + def test_name_only_is_not_custom(self) -> None: + assert not ProviderSettings(name="copilot").has_custom_routing() + + @pytest.mark.parametrize( + "field,value", + [ + # Anchor fields (any one activates custom routing on its own). + ("base_url", "http://x"), + ("api_key", "k"), + ("bearer_token", "t"), + ], + ) + def test_anchor_field_activates_custom_routing(self, field: str, value: object) -> None: + s = ProviderSettings(name="copilot", **{field: value}) # type: ignore[arg-type] + assert s.has_custom_routing() + + @pytest.mark.parametrize( + "field,value", + [ + # Non-anchor fields activate custom routing only alongside an anchor; + # the schema validator rejects them on their own (see TestProviderSettingsValidation). + ("type", "openai"), + ("wire_api", "completions"), + ("headers", {"X-Foo": "1"}), + ], + ) + def test_non_anchor_field_activates_with_anchor(self, field: str, value: object) -> None: + s = ProviderSettings(name="copilot", base_url="http://x/v1", **{field: value}) # type: ignore[arg-type] + assert s.has_custom_routing() + + def test_azure_activates_custom_routing(self) -> None: + s = ProviderSettings( + name="copilot", + type="azure", + base_url="https://x.openai.azure.com", + azure=AzureProviderOptions(api_version="2024-10-21"), + ) + assert s.has_custom_routing() diff --git a/tests/test_config/test_schema.py b/tests/test_config/test_schema.py index bba49941..06734dda 100644 --- a/tests/test_config/test_schema.py +++ b/tests/test_config/test_schema.py @@ -375,7 +375,8 @@ class TestRuntimeConfig: def test_default_values(self) -> None: """Test default runtime configuration.""" config = RuntimeConfig() - assert config.provider == "copilot" + assert config.provider.name == "copilot" + assert not config.provider.has_custom_routing() assert config.default_model is None assert config.temperature is None assert config.max_tokens is None @@ -384,7 +385,7 @@ def test_default_values(self) -> None: def test_custom_provider(self) -> None: """Test custom provider setting.""" config = RuntimeConfig(provider="openai-agents", default_model="gpt-4") - assert config.provider == "openai-agents" + assert config.provider.name == "openai-agents" assert config.default_model == "gpt-4" def test_invalid_provider_raises(self) -> None: @@ -395,7 +396,7 @@ def test_invalid_provider_raises(self) -> None: def test_claude_provider_with_temperature(self) -> None: """Test Claude provider with temperature setting.""" config = RuntimeConfig(provider="claude", temperature=0.7) - assert config.provider == "claude" + assert config.provider.name == "claude" assert config.temperature == 0.7 def test_temperature_boundary_values(self) -> None: @@ -463,7 +464,7 @@ def test_all_common_fields_together(self) -> None: max_tokens=4096, timeout=120.0, ) - assert config.provider == "claude" + assert config.provider.name == "claude" assert config.default_model == "claude-3-5-sonnet-latest" assert config.temperature == 0.7 assert config.max_tokens == 4096 @@ -588,7 +589,7 @@ def test_minimal_workflow(self) -> None: workflow = WorkflowDef(name="test", entry_point="agent1") assert workflow.name == "test" assert workflow.entry_point == "agent1" - assert workflow.runtime.provider == "copilot" + assert workflow.runtime.provider.name == "copilot" def test_full_workflow(self) -> None: """Test fully configured workflow definition.""" diff --git a/tests/test_integration/test_end_to_end_claude.py b/tests/test_integration/test_end_to_end_claude.py index a31dc64f..51e8b8f7 100644 --- a/tests/test_integration/test_end_to_end_claude.py +++ b/tests/test_integration/test_end_to_end_claude.py @@ -63,7 +63,7 @@ async def test_basic_claude_workflow_execution(self, tmp_path, mock_anthropic_re # Load and validate workflow config = load_workflow(str(workflow_yaml)) - assert config.workflow.runtime.provider == "claude" + assert config.workflow.runtime.provider.name == "claude" assert config.workflow.runtime.temperature == 0.7 assert config.workflow.runtime.max_tokens == 1000 @@ -267,7 +267,7 @@ async def test_backward_compatibility_in_workflow(self, tmp_path, mock_anthropic """) config = load_workflow(str(workflow_yaml)) - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" # Verify no Claude fields leaked assert config.workflow.runtime.temperature is None diff --git a/tests/test_integration/test_mixed_providers.py b/tests/test_integration/test_mixed_providers.py index 81f22de1..b5ef2fe0 100644 --- a/tests/test_integration/test_mixed_providers.py +++ b/tests/test_integration/test_mixed_providers.py @@ -46,7 +46,7 @@ def test_workflow_has_single_provider(self, tmp_path): """) config = load_workflow(str(workflow_yaml)) - assert config.workflow.runtime.provider == "claude" + assert config.workflow.runtime.provider.name == "claude" def test_can_override_provider_per_agent(self, tmp_path): """Verify agent-level provider override is now supported.""" @@ -72,7 +72,7 @@ def test_can_override_provider_per_agent(self, tmp_path): config = load_workflow(str(workflow_yaml)) # Workflow default is copilot - assert config.workflow.runtime.provider == "copilot" + assert config.workflow.runtime.provider.name == "copilot" # Agent overrides to claude assert config.agents[0].provider == "claude" # Agent schema now has 'provider' field @@ -438,4 +438,4 @@ def test_null_provider_uses_default(self, tmp_path) -> None: """) config = load_workflow(str(workflow_yaml)) assert config.agents[0].provider is None - assert config.workflow.runtime.provider == "claude" + assert config.workflow.runtime.provider.name == "claude" diff --git a/tests/test_providers/test_copilot_provider_routing.py b/tests/test_providers/test_copilot_provider_routing.py new file mode 100644 index 00000000..90043e14 --- /dev/null +++ b/tests/test_providers/test_copilot_provider_routing.py @@ -0,0 +1,525 @@ +"""Tests for structured ``runtime.provider`` plumbing through ``CopilotProvider``. + +Covers issue #136: the resolver (env-var fallbacks, activation gate, secret +unwrap) and the central ``_apply_provider_config`` plumbing into both the +main agent session and dialog turns. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock + +import pytest +from pydantic import SecretStr + +from conductor.config.schema import ( + AzureProviderOptions, + ProviderSettings, + RuntimeConfig, +) +from conductor.providers.copilot import CopilotProvider + + +def _make_provider(**kwargs: Any) -> CopilotProvider: + return CopilotProvider(**kwargs) + + +class TestResolveSdkProviderConfig: + """Unit-tests for ``CopilotProvider._resolve_sdk_provider_config``.""" + + def test_no_settings_returns_none(self) -> None: + provider = _make_provider() + assert provider._resolve_sdk_provider_config() is None + + def test_name_only_returns_none(self) -> None: + """Default routing (no custom fields) must not forward a provider + dict to the SDK — that would silently activate based on ambient + OpenAI env vars.""" + provider = _make_provider(provider_settings=ProviderSettings(name="copilot")) + assert provider._resolve_sdk_provider_config() is None + + def test_env_vars_alone_do_not_activate(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Ambient ``OPENAI_*`` env vars must not divert default Copilot traffic.""" + monkeypatch.setenv("OPENAI_BASE_URL", "http://env-host/v1") + monkeypatch.setenv("OPENAI_API_KEY", "sk-from-env") + provider = _make_provider(provider_settings=ProviderSettings(name="copilot")) + assert provider._resolve_sdk_provider_config() is None + + def test_full_yaml_config_passes_through(self) -> None: + s = ProviderSettings.model_validate( + { + "name": "copilot", + "type": "openai", + "wire_api": "completions", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-yaml", + } + ) + provider = _make_provider(provider_settings=s, model="ollama/llama3") + cfg = provider._resolve_sdk_provider_config() + assert cfg == { + "type": "openai", + "wire_api": "completions", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-yaml", + } + + def test_yaml_base_url_then_env_api_key(self, monkeypatch: pytest.MonkeyPatch) -> None: + """YAML opts in; ``COPILOT_PROVIDER_API_KEY`` fills missing + ``api_key``. Ambient ``OPENAI_API_KEY`` is intentionally NOT + used as a fallback (credential-leak risk).""" + monkeypatch.setenv("COPILOT_PROVIDER_API_KEY", "sk-copilot-env") + # Ambient OPENAI_API_KEY must NOT be consulted. + monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-should-not-leak") + s = ProviderSettings(name="copilot", base_url="http://yaml/v1") + provider = _make_provider(provider_settings=s, model="custom-model") + cfg = provider._resolve_sdk_provider_config() + assert cfg == { + "type": "openai", # defaulted because base_url is set + "base_url": "http://yaml/v1", + "api_key": "sk-copilot-env", + } + + def test_openai_api_key_is_not_an_ambient_fallback( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Regression: ambient ``OPENAI_API_KEY`` must not leak into the + SDK provider config — only the namespaced + ``COPILOT_PROVIDER_API_KEY`` is a fallback.""" + monkeypatch.delenv("COPILOT_PROVIDER_API_KEY", raising=False) + monkeypatch.setenv("OPENAI_API_KEY", "sk-do-not-leak") + s = ProviderSettings(name="copilot", base_url="http://ollama/v1") + provider = _make_provider(provider_settings=s, model="m") + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert "api_key" not in cfg + + def test_copilot_provider_env_var_takes_precedence_over_openai( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """``COPILOT_PROVIDER_BASE_URL`` beats ``OPENAI_BASE_URL``.""" + monkeypatch.setenv("OPENAI_BASE_URL", "http://openai/v1") + monkeypatch.setenv("COPILOT_PROVIDER_BASE_URL", "http://copilot-env/v1") + monkeypatch.setenv("COPILOT_PROVIDER_API_KEY", "copilot-env-key") + s = ProviderSettings(name="copilot", api_key="anchor-key", wire_api="completions") + provider = _make_provider(provider_settings=s, model="m") + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert cfg["base_url"] == "http://copilot-env/v1" + # api_key in YAML wins over env fallback. + assert cfg["api_key"] == "anchor-key" + + def test_openai_base_url_fallback_alone(self, monkeypatch: pytest.MonkeyPatch) -> None: + """``OPENAI_BASE_URL`` IS a recognized fallback (URLs are not + secrets) — but only after YAML opts in via some other field.""" + monkeypatch.delenv("COPILOT_PROVIDER_BASE_URL", raising=False) + monkeypatch.setenv("OPENAI_BASE_URL", "http://env-only/v1") + s = ProviderSettings(name="copilot", api_key="anchor-key", type="openai") + provider = _make_provider(provider_settings=s, model="m") + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert cfg["base_url"] == "http://env-only/v1" + + def test_bearer_token_from_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("COPILOT_PROVIDER_BEARER_TOKEN", "bearer-from-env") + s = ProviderSettings(name="copilot", base_url="http://x/v1") + provider = _make_provider(provider_settings=s, model="m") + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert cfg["bearer_token"] == "bearer-from-env" + + def test_azure_options_nested(self) -> None: + s = ProviderSettings( + name="copilot", + type="azure", + base_url="https://x.openai.azure.com", + api_key="azure-key", + azure=AzureProviderOptions(api_version="2024-10-21"), + ) + provider = _make_provider(provider_settings=s, model="gpt-4o") + cfg = provider._resolve_sdk_provider_config() + assert cfg == { + "type": "azure", + "base_url": "https://x.openai.azure.com", + "api_key": "azure-key", + "azure": {"api_version": "2024-10-21"}, + } + + def test_headers_passed_through(self) -> None: + s = ProviderSettings( + name="copilot", + type="openai", + base_url="http://x/v1", + headers={"X-Custom": "yes", "Authorization": "Bearer foo"}, + ) + provider = _make_provider(provider_settings=s, model="m") + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert cfg["headers"] == {"X-Custom": "yes", "Authorization": "Bearer foo"} + + def test_dual_credentials_in_yaml_warns(self, caplog: pytest.LogCaptureFixture) -> None: + s = ProviderSettings( + name="copilot", + base_url="http://x/v1", + api_key="k", + bearer_token="t", + ) + provider = _make_provider(provider_settings=s, model="m") + with caplog.at_level("WARNING", logger="conductor.providers.copilot"): + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert cfg.get("api_key") == "k" + assert cfg.get("bearer_token") == "t" + assert any("bearer_token" in r.message for r in caplog.records), ( + "expected a warning about dual api_key+bearer_token" + ) + + def test_dual_credentials_yaml_plus_env_warns( + self, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture + ) -> None: + """Regression for silent-failures #5: the dual-credential warning + must fire on YAML × env mixing (api_key in YAML, + ``COPILOT_PROVIDER_BEARER_TOKEN`` in env), not just YAML-only. + """ + monkeypatch.setenv("COPILOT_PROVIDER_BEARER_TOKEN", "bearer-from-env") + s = ProviderSettings(name="copilot", base_url="http://x/v1", api_key="yaml-key") + provider = _make_provider(provider_settings=s, model="m") + with caplog.at_level("WARNING", logger="conductor.providers.copilot"): + cfg = provider._resolve_sdk_provider_config() + assert cfg is not None + assert cfg["api_key"] == "yaml-key" + assert cfg["bearer_token"] == "bearer-from-env" + assert any("bearer_token" in r.message for r in caplog.records), ( + "expected a warning when api_key (YAML) and bearer_token (env) both resolve" + ) + + def test_raises_when_routing_resolves_to_empty(self, monkeypatch: pytest.MonkeyPatch) -> None: + """Regression for silent-failures #1: when has_custom_routing() + is True but every resolved field is falsy (e.g. all expected env + vars are unset), raise a clear error instead of silently + dropping the SDK provider kwarg.""" + from conductor.exceptions import ProviderError + + # All COPILOT_PROVIDER_* and OPENAI_BASE_URL absent. + for var in ( + "COPILOT_PROVIDER_BASE_URL", + "OPENAI_BASE_URL", + "COPILOT_PROVIDER_API_KEY", + "COPILOT_PROVIDER_BEARER_TOKEN", + ): + monkeypatch.delenv(var, raising=False) + + # bearer_token from YAML is *intentionally* the only field set; + # then bypass schema validation via model_construct to simulate a + # caller (or future schema bug) that lets an empty SecretStr through. + s = ProviderSettings.model_construct(name="copilot", bearer_token=SecretStr("")) + provider = _make_provider(provider_settings=s, model="m") + with pytest.raises(ProviderError, match="no usable fields"): + provider._resolve_sdk_provider_config() + + def test_default_model_warning_on_custom_routing( + self, caplog: pytest.LogCaptureFixture + ) -> None: + s = ProviderSettings(name="copilot", base_url="http://ollama/v1") + with caplog.at_level("WARNING", logger="conductor.providers.copilot"): + _make_provider(provider_settings=s) # no model kwarg → warn + msgs = [r.message.lower() for r in caplog.records] + assert any("custom copilot provider routing" in m for m in msgs), msgs + + def test_no_default_model_warning_when_user_supplies_model( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Regression for silent-failures #4: the warning must be tied + to *whether the caller supplied a model*, not to a magic-string + comparison against the SDK fallback. A user who explicitly + picks the same model name as the fallback must NOT see the + warning.""" + s = ProviderSettings(name="copilot", base_url="http://ollama/v1") + with caplog.at_level("WARNING", logger="conductor.providers.copilot"): + _make_provider(provider_settings=s, model="gpt-4o") + msgs = [r.message.lower() for r in caplog.records] + assert not any("custom copilot provider routing" in m for m in msgs), ( + "explicit model should suppress the default-model warning even " + "when it matches the SDK fallback" + ) + + +class TestApplyProviderConfig: + """``_apply_provider_config`` mutates session kwargs in place.""" + + def test_no_settings_leaves_kwargs_unchanged(self) -> None: + provider = _make_provider() + kwargs: dict[str, Any] = {"model": "gpt-4o"} + provider._apply_provider_config(kwargs) + assert "provider" not in kwargs + + def test_custom_routing_attaches_provider_dict(self) -> None: + s = ProviderSettings(name="copilot", base_url="http://x/v1", api_key="k") + provider = _make_provider(provider_settings=s, model="m") + kwargs: dict[str, Any] = {"model": "m"} + provider._apply_provider_config(kwargs) + assert kwargs["provider"] == { + "type": "openai", + "base_url": "http://x/v1", + "api_key": "k", + } + + +class TestSessionKwargsPlumbing: + """End-to-end: provider config reaches ``create_session`` in both + main agent execution and dialog turns.""" + + def _build_mocked_provider( + self, captured: dict[str, Any], provider_settings: ProviderSettings | None + ) -> CopilotProvider: + provider = CopilotProvider(provider_settings=provider_settings, model="custom-model") + provider._started = True + + session = AsyncMock() + captured_callback: dict[str, Any] = {} + + def on_event(callback: Any) -> None: + captured_callback["cb"] = callback + + session.on = on_event + + async def send(prompt: str) -> None: + captured["sent_prompt"] = prompt + cb = captured_callback["cb"] + # Synthesize a minimal "assistant.message" + "session.idle" pair + from types import SimpleNamespace + + def make_event(t: str, content: str = "") -> Any: + ev = SimpleNamespace() + ev.type = SimpleNamespace(value=t) + ev.data = SimpleNamespace(message=content, content=content) + return ev + + cb(make_event("assistant.message", "ok")) + cb(make_event("session.idle")) + + session.send = send + session.destroy = AsyncMock() + + async def create_session(**kwargs: Any) -> Any: + captured["create_session_kwargs"] = kwargs + return session + + client = AsyncMock() + client.create_session = create_session + # resume_session is never called when no resume id is set + provider._client = client + return provider + + @pytest.mark.asyncio + async def test_dialog_turn_attaches_provider_config(self) -> None: + s = ProviderSettings( + name="copilot", + type="openai", + wire_api="completions", + base_url="http://localhost:11434/v1", + api_key="sk-dialog", + ) + captured: dict[str, Any] = {} + provider = self._build_mocked_provider(captured, s) + await provider.execute_dialog_turn( + system_prompt="be helpful", + user_message="hi", + history=[], + ) + kwargs = captured["create_session_kwargs"] + assert kwargs["provider"] == { + "type": "openai", + "wire_api": "completions", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-dialog", + } + + @pytest.mark.asyncio + async def test_dialog_turn_no_provider_when_default(self) -> None: + """Default routing (no structured settings) means no provider + kwarg to ``create_session`` — preserves out-of-the-box SDK + behavior.""" + captured: dict[str, Any] = {} + provider = self._build_mocked_provider(captured, None) + await provider.execute_dialog_turn( + system_prompt="be helpful", + user_message="hi", + history=[], + ) + kwargs = captured["create_session_kwargs"] + assert "provider" not in kwargs + + @pytest.mark.asyncio + async def test_execute_attaches_provider_config(self) -> None: + """Parallel to ``test_dialog_turn_attaches_provider_config``: + confirm that the main agent execution path also attaches the + resolved ``ProviderConfig`` dict. A regression that removes + the ``_apply_provider_config`` call from ``_execute_sdk_call`` + but keeps it in the dialog path would otherwise silently ship. + """ + from conductor.config.schema import AgentDef + + s = ProviderSettings( + name="copilot", + type="openai", + wire_api="completions", + base_url="http://localhost:11434/v1", + api_key="sk-execute", + ) + captured: dict[str, Any] = {} + provider = self._build_mocked_provider(captured, s) + + agent = AgentDef(name="solo", model="custom-model", prompt="hi") + await provider.execute(agent, context={}, rendered_prompt="hi") + + kwargs = captured["create_session_kwargs"] + assert kwargs["provider"] == { + "type": "openai", + "wire_api": "completions", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-execute", + } + + +class TestDescribeProviderRedaction: + """``_describe_provider`` must never leak ``SecretStr`` values.""" + + def test_secret_values_never_appear_in_output(self) -> None: + """Regression: a refactor that swaps ``_describe_provider`` for + ``str(provider)`` or inlines ``provider.api_key.get_secret_value()`` + must fail this test rather than silently leak credentials to + verbose logs / event sinks.""" + from conductor.cli.run import _describe_provider + + secret_api_key = "sk-DO-NOT-LEAK-12345" + secret_bearer = "bearer-DO-NOT-LEAK-67890" + secret_header = "Bearer secret-header-token-abcdef" + + s = ProviderSettings( + name="copilot", + type="openai", + base_url="http://localhost:11434/v1", + api_key=secret_api_key, + bearer_token=secret_bearer, + headers={"Authorization": secret_header, "X-Trace": "1"}, + ) + rendered = _describe_provider(s) + + # Identifying metadata appears, redacted markers appear. + assert "copilot" in rendered + assert "type=openai" in rendered + assert "base_url=http://localhost:11434/v1" in rendered + assert "api_key=***" in rendered + assert "bearer_token=***" in rendered + + # The secret values themselves never appear. + assert secret_api_key not in rendered + assert secret_bearer not in rendered + # Header values are never rendered (only keys are listed). + assert secret_header not in rendered + + def test_default_routing_renders_bare_name(self) -> None: + from conductor.cli.run import _describe_provider + + assert _describe_provider(ProviderSettings(name="copilot")) == "copilot" + + +class TestRegistryForwardsSettings: + """``ProviderRegistry`` forwards ``ProviderSettings`` only to the matching provider.""" + + @pytest.mark.asyncio + async def test_registry_forwards_settings_to_copilot( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + from conductor.config.schema import AgentDef, WorkflowConfig, WorkflowDef + from conductor.providers.registry import ProviderRegistry + + captured: dict[str, Any] = {} + + async def fake_create_provider(**kwargs: Any) -> Any: + captured.update(kwargs) + mock = AsyncMock() + mock.set_resume_session_ids = lambda *_a, **_k: None + return mock + + monkeypatch.setattr("conductor.providers.registry.create_provider", fake_create_provider) + + runtime = RuntimeConfig.model_validate( + { + "provider": { + "name": "copilot", + "base_url": "http://localhost:11434/v1", + "type": "openai", + } + } + ) + config = WorkflowConfig( + workflow=WorkflowDef(name="test", entry_point="a", runtime=runtime), + agents=[AgentDef(name="a", prompt="p")], + output={"r": "x"}, + ) + + async with ProviderRegistry(config) as registry: + await registry.get_provider(config.agents[0]) + + assert captured["provider_type"] == "copilot" + assert captured["provider_settings"] is runtime.provider + assert captured["provider_settings"].base_url == "http://localhost:11434/v1" + + @pytest.mark.asyncio + async def test_registry_isolates_copilot_settings_from_claude_override( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Regression for the inline guard in ``registry.py``: when the + workflow default is structured Copilot routing but an agent + overrides to ``claude``, the registry must pass + ``provider_settings=None`` to the Claude factory call. + Otherwise Copilot-shaped routing (``bearer_token`` / ``headers`` + / etc.) would silently bleed into the Claude provider. + """ + from conductor.config.schema import AgentDef, WorkflowConfig, WorkflowDef + from conductor.providers.registry import ProviderRegistry + + captured: list[dict[str, Any]] = [] + + async def fake_create_provider(**kwargs: Any) -> Any: + captured.append(dict(kwargs)) + mock = AsyncMock() + mock.set_resume_session_ids = lambda *_a, **_k: None + return mock + + monkeypatch.setattr("conductor.providers.registry.create_provider", fake_create_provider) + + runtime = RuntimeConfig.model_validate( + { + "provider": { + "name": "copilot", + "type": "openai", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-copilot-only", + } + } + ) + config = WorkflowConfig( + workflow=WorkflowDef(name="test", entry_point="copilot_agent", runtime=runtime), + agents=[ + AgentDef(name="copilot_agent", prompt="p"), + AgentDef(name="claude_agent", prompt="p", provider="claude"), + ], + output={"r": "x"}, + ) + + async with ProviderRegistry(config) as registry: + await registry.get_provider(config.agents[0]) # copilot + await registry.get_provider(config.agents[1]) # claude override + + by_type = {call["provider_type"]: call for call in captured} + assert by_type.keys() == {"copilot", "claude"} + + # Copilot call DOES receive the structured settings. + assert by_type["copilot"]["provider_settings"] is runtime.provider + + # Claude call MUST NOT receive Copilot-shaped settings. + assert by_type["claude"]["provider_settings"] is None diff --git a/tests/test_providers/test_registry.py b/tests/test_providers/test_registry.py index 290ac3f5..b53f68b7 100644 --- a/tests/test_providers/test_registry.py +++ b/tests/test_providers/test_registry.py @@ -331,6 +331,7 @@ async def test_runtime_config_passed_to_provider(self, mock_create: MagicMock) - timeout=60.0, max_session_seconds=None, max_agent_iterations=None, + provider_settings=config.workflow.runtime.provider, ) @patch("conductor.providers.registry.create_provider")