feat(providers): structured runtime.provider config for Copilot custom endpoints (closes #136)#225
Open
jrob5756 wants to merge 1 commit into
Open
feat(providers): structured runtime.provider config for Copilot custom endpoints (closes #136)#225jrob5756 wants to merge 1 commit into
jrob5756 wants to merge 1 commit into
Conversation
2c6e818 to
e1416b4
Compare
Collaborator
Author
Review fixes round 1 (critical + high)Force-pushed Critical (silent failures that masqueraded as default behavior):
High:
Deferred to follow-ups (called out in commit msg):
Verification: |
e1416b4 to
8d62c5f
Compare
…m endpoints 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>
8d62c5f to
423bca2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #136.
Summary
runtime.providernow accepts either the bare string shorthand(
provider: copilot, current behavior, unchanged) or a structuredobject that forwards a
ProviderConfigto the Copilot SDK'screate_session(provider=…)parameter. This lets workflows route theCopilot SDK at OpenAI-compatible / Azure / Anthropic endpoints (Ollama,
vLLM, LM Studio, Azure OpenAI, etc.) instead of being locked to the
GitHub Copilot service.
Scope agreed with the reporter: Copilot provider only this PR,
YAML + env-var fallbacks (no new CLI flags), and validate
type/
wire_apiagainst the SDK's supported set.Design highlights
providerkwarg is only attached whenYAML explicitly opts in (at least one field beyond
name). AmbientOPENAI_*env vars never silently divert default Copilottraffic.
base_url←COPILOT_PROVIDER_BASE_URL→OPENAI_BASE_URLapi_key←COPILOT_PROVIDER_API_KEY→OPENAI_API_KEYbearer_token←COPILOT_PROVIDER_BEARER_TOKENtypedefaults toopenaiwhenbase_urlis setapi_keyandbearer_tokenareSecretStr,so they redact in
model_dump/workflow_startedpayloads /dashboard / event logs. A redacted
_describe_provider()helperfeeds verbose logs.
_apply_provider_config()is called from both_execute_sdk_call(agents) andexecute_dialog_turn(dialog mode)so all sessions hit the same endpoint.
type/wire_apiareLiteral[...](Pydanticenforces);
azureoptions requiretype: azure;name != "copilot"rejects any Copilot-only field (structuredconfig for other providers is explicit follow-up).
default
gpt-4o, so the provider warns when custom routing isactive without
runtime.default_model.--provider <name>replaces the entireProviderSettings; logs a notice when YAML had structured fields.Wired into both
runandresumefor parity.model_serializercollapsesProviderSettingsback to a bare string when onlynameis set,so serialized YAML/JSON stays byte-compatible with the prior schema.
Files
src/conductor/config/schema.pyProviderSettings,AzureProviderOptions;RuntimeConfigreworksrc/conductor/providers/copilot.pyprovider_settingskwarg,_resolve_sdk_provider_config,_apply_provider_config, applied to agent + dialog pathssrc/conductor/providers/factory.pyprovider_settings(Copilot only)src/conductor/providers/registry.pyruntime.providerto factory only on matchingnamesrc/conductor/cli/run.pyresumeexamples/copilot-local-llm.yamlAGENTS.mdtests/test_config/test_provider_settings.pytests/test_providers/test_copilot_provider_routing.pyTest plan
make lint✅make typecheck✅ (only pre-existing diagnostic remains)make validate-examples✅uv run pytest tests/ -m "not performance" -q→ 2817 passed, 16 skipped, 29 deselected ✅Out of scope (called out for follow-up)
claude/openai-agents.resumecan warn on configuration drift.
--provider-base-url).Credit
Inspired by @epowers's proof-of-concept
patch
attached to the issue.