From 36f92e1be270bbdb45e8969681ee1bf5c4ce44ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:22:15 +0000 Subject: [PATCH 1/4] Add Copilot external-runtime connection (runtime_url/runtime_token + env) --- src/conductor/config/schema.py | 83 +++++++++++++++++++++++++++++- src/conductor/providers/copilot.py | 70 ++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 2 deletions(-) diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 9622b007..cc091fcc 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -1711,6 +1711,40 @@ class ProviderSettings(BaseModel): """Azure-specific options (e.g. ``api_version``). Requires ``type: azure``. Copilot-only.""" + runtime_url: str | None = None + """Connect to an already-running Copilot runtime instead of spawning a + nested one. Copilot-only. + + Accepts ``"port"``, ``"host:port"``, or a full URL. When set, the Copilot + provider connects to the external runtime via the SDK's + ``RuntimeConnection.for_uri(...)`` — no child ``copilot`` process is + spawned, so every agent/model reuses the server's single authenticated + session. This is the recommended way to run Conductor inside an external + orchestrator (e.g. Agency) that already owns an authenticated + ``copilot --server`` process. + + Falls back to the ``COPILOT_PROVIDER_RUNTIME_URL`` environment variable + when not set in YAML. Mutually exclusive with custom endpoint routing + (``base_url`` / ``api_key`` / ``bearer_token`` / ``type`` / ``wire_api`` / + ``headers`` / ``azure``): the external runtime *is* the endpoint. + + Example:: + + provider: + name: copilot + runtime_url: localhost:3000 + runtime_token: ${COPILOT_RUNTIME_TOKEN} + """ + + runtime_token: SecretStr | None = None + """Shared secret authenticating the connection to ``runtime_url``. Copilot-only. + + Required when the server was started with a connection token. Prefer + ``${COPILOT_RUNTIME_TOKEN}`` interpolation so the literal value never lands + in ``workflow_started`` events or checkpoints. Falls back to the + ``COPILOT_PROVIDER_RUNTIME_TOKEN`` environment variable when not set in + YAML. Requires ``runtime_url``.""" + hermes_home: str | None = None """Path to a Hermes home directory (profile). Hermes-only. @@ -1756,6 +1790,8 @@ def _check_field_compatibility(self) -> ProviderSettings: "bearer_token": self.bearer_token, "headers": self.headers, "azure": self.azure, + "runtime_url": self.runtime_url, + "runtime_token": self.runtime_token, } claude_only_fields = { "auth_token": self.auth_token, @@ -1806,6 +1842,7 @@ def _check_field_compatibility(self) -> ProviderSettings: ("api_key", self.api_key), ("bearer_token", self.bearer_token), ("auth_token", self.auth_token), + ("runtime_token", self.runtime_token), ): if value is not None and value.get_secret_value() == "": raise ValueError( @@ -1837,6 +1874,35 @@ def _check_field_compatibility(self) -> ProviderSettings: "'azure' block is empty; either set azure.api_version or remove the block" ) + # Connecting to an external runtime (runtime_url) is mutually + # exclusive with custom endpoint routing — the runtime *is* the + # endpoint, so a base_url / api_key / etc. alongside it is + # contradictory and would silently be ignored at the SDK boundary. + if self.runtime_url is not None: + routing_conflicts = sorted( + k + for k, v in { + "base_url": self.base_url, + "api_key": self.api_key, + "bearer_token": self.bearer_token, + "type": self.type, + "wire_api": self.wire_api, + "headers": self.headers, + "azure": self.azure, + }.items() + if v is not None + ) + if routing_conflicts: + raise ValueError( + f"'runtime_url' cannot be combined with {routing_conflicts}; connecting " + "to an existing runtime and custom endpoint routing are mutually exclusive " + "(the runtime is the endpoint)." + ) + + # A connection token is meaningless without a URL to connect to. + if self.runtime_token is not None and self.runtime_url is None: + raise ValueError("'runtime_token' requires 'runtime_url' to also be set") + return self def has_custom_routing(self) -> bool: @@ -1846,6 +1912,11 @@ def has_custom_routing(self) -> bool: set. We never activate from ambient environment variables alone — that would silently divert default Copilot traffic based on unrelated shell state. + + Note: this covers only *endpoint* routing (``base_url`` and friends). + Connecting to an existing runtime (``runtime_url``) is a separate + axis — see :meth:`has_external_runtime` — and is intentionally + excluded so it does not activate the endpoint-provider resolver. """ return any( value is not None @@ -1861,6 +1932,16 @@ def has_custom_routing(self) -> bool: ) ) + def has_external_runtime(self) -> bool: + """Return True when YAML configured connecting to an existing runtime. + + Gated on ``runtime_url`` being set in YAML. As with custom routing, + ambient environment variables never activate this on their own here; + the ``COPILOT_PROVIDER_RUNTIME_URL`` fallback is resolved at the + provider layer. + """ + return self.runtime_url is not None + @model_serializer(mode="wrap") def _serialize(self, nxt: Any) -> Any: """Collapse to bare string when only ``name`` is set. @@ -1871,7 +1952,7 @@ def _serialize(self, nxt: Any) -> Any: not as ``{"name": "copilot"}``. Once any structured field is set, the full object is emitted. """ - if not self.has_custom_routing(): + if not self.has_custom_routing() and not self.has_external_runtime(): return self.name return nxt(self) diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index ff220f37..c9b4e0e6 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -79,12 +79,14 @@ def _is_finite_nonneg(value: object) -> TypeGuard[float]: # Try to import the Copilot SDK try: from copilot import CopilotClient + from copilot.client import RuntimeConnection from copilot.session import PermissionHandler COPILOT_SDK_AVAILABLE = True except ImportError: COPILOT_SDK_AVAILABLE = False CopilotClient = None # type: ignore[misc, assignment] + RuntimeConnection = None # type: ignore[misc, assignment] PermissionHandler = None # type: ignore[misc, assignment] @@ -459,6 +461,41 @@ def _apply_provider_config(self, session_kwargs: dict[str, Any]) -> None: if provider_cfg is not None: session_kwargs["provider"] = provider_cfg + def _resolve_runtime_connection(self) -> tuple[str, str | None] | None: + """Resolve whether to connect to an already-running Copilot runtime. + + Returns ``(url, connection_token)`` when Conductor should connect to + an external runtime instead of spawning its own child process, or + ``None`` for the default (spawn) behavior. + + Resolution order for each field is YAML (``runtime.provider``) first, + then a namespaced environment variable: + + - ``url``: ``runtime_url`` → ``COPILOT_PROVIDER_RUNTIME_URL`` + - ``token``: ``runtime_token`` → ``COPILOT_PROVIDER_RUNTIME_TOKEN`` + + The environment variables activate the connection on their own (no + YAML required), which is the intended zero-config path for external + orchestrators such as Agency: they launch one authenticated + ``copilot --server`` and export these two variables. The variables are + namespaced (``COPILOT_PROVIDER_*``) specifically so unrelated ambient + shell state cannot silently divert default Copilot traffic. + """ + settings = self._provider_settings + + url = settings.runtime_url if settings is not None else None + url = url or os.environ.get("COPILOT_PROVIDER_RUNTIME_URL") + + token: str | None = None + if settings is not None and settings.runtime_token is not None: + token = settings.runtime_token.get_secret_value() + if token is None: + token = os.environ.get("COPILOT_PROVIDER_RUNTIME_TOKEN") + + if not url: + return None + return (url, token or None) + async def execute( self, agent: AgentDef, @@ -2017,7 +2054,7 @@ async def _ensure_client_started(self) -> None: """ async with self._start_lock: if self._client is None: - self._client = CopilotClient() + self._client = self._build_client() if not self._started: await self._client.start() self._started = True @@ -2027,6 +2064,37 @@ async def _ensure_client_started(self) -> None: # may set O_NONBLOCK on inherited file descriptors. self._fix_pipe_blocking_mode() + def _build_client(self) -> Any: + """Construct the Copilot SDK client. + + When a runtime connection is resolved (via ``runtime_url`` / + ``COPILOT_PROVIDER_RUNTIME_URL``), connect to that already-running + runtime instead of spawning a nested ``copilot`` child process. The + SDK's ``start()`` skips process spawning for URI connections and its + ``stop()`` leaves the externally-owned server running, so Conductor + reuses the server's single authenticated session for every agent. + """ + connection = self._resolve_runtime_connection() + if connection is None: + return CopilotClient() + + url, token = connection + if self._provider_settings is not None and self._provider_settings.has_custom_routing(): + logger.warning( + "Both an external runtime connection (runtime_url=%s) and custom endpoint " + "routing are configured; the custom endpoint routing will still be applied " + "to sessions on the external runtime. These are usually mutually exclusive — " + "verify this is intended.", + url, + ) + logger.info( + "Connecting to existing Copilot runtime at %s (no nested runtime spawned)", + url, + ) + return CopilotClient( + connection=RuntimeConnection.for_uri(url, connection_token=token) + ) + def _fix_pipe_blocking_mode(self) -> None: """Clear O_NONBLOCK on the Copilot CLI subprocess pipes. From 4bca623ac7b48fb0697f8e8a5ca6c245fcd98612 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:27:14 +0000 Subject: [PATCH 2/4] Add docs, example, tests for Copilot existing-runtime connection --- AGENTS.md | 3 +- CHANGELOG.md | 11 +++ docs/configuration.md | 77 +++++++++++++++++ examples/copilot-existing-runtime.yaml | 83 +++++++++++++++++++ src/conductor/providers/copilot.py | 4 +- tests/test_config/test_provider_settings.py | 80 ++++++++++++++++++ .../test_copilot_provider_routing.py | 67 +++++++++++++++ 7 files changed, 321 insertions(+), 4 deletions(-) create mode 100644 examples/copilot-existing-runtime.yaml diff --git a/AGENTS.md b/AGENTS.md index d7215442..2f14c71b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,7 +111,7 @@ step-by-step checklist. - **providers/**: SDK provider abstraction - `base.py` - `AgentProvider` ABC defining `execute()`, `validate_connection()`, `close()` - - `copilot.py` - GitHub Copilot SDK implementation + - `copilot.py` - GitHub Copilot SDK implementation. By default spawns a nested `copilot` runtime via `CopilotClient()` (in `_build_client`, called from `_ensure_client_started`). When a runtime connection is resolved (`runtime.provider.runtime_url` or `COPILOT_PROVIDER_RUNTIME_URL`, optional `runtime_token` / `COPILOT_PROVIDER_RUNTIME_TOKEN`), it instead builds `CopilotClient(connection=RuntimeConnection.for_uri(url, connection_token=token))` to connect to an already-running `copilot --server`; the SDK skips spawning for URI connections and its `stop()` leaves the externally-owned server running. `_resolve_runtime_connection()` reads YAML first, then the namespaced env var (env activates on its own — the zero-YAML path for orchestrators like Agency). Mutually exclusive with custom endpoint routing. - `claude.py` - Anthropic Claude API implementation - `claude_agent_sdk.py` - Claude Agent SDK implementation (uses `claude-agent-sdk` package) - `factory.py` - Provider instantiation @@ -154,6 +154,7 @@ step-by-step checklist. - **Periodic checkpoints** (`runtime.checkpoint`, issue #244): opt-in `CheckpointConfig` (`every_agent: bool`, `every_seconds: int|None`, `keep_last: int=5`; `is_enabled = every_agent or every_seconds is not None`). Off by default → failure-only behavior preserved. `WorkflowEngine._maybe_save_periodic_checkpoint()` is called once at the **top of `_execute_loop`** (single choke point), where prior outputs are committed and `_current_agent_name` is the step *about to run* — so a periodic checkpoint reuses failure-checkpoint `current_agent` semantics and resume continues forward with no special-casing. Gated via the `_periodic_checkpoints_active` property (**root engine only**, `_subworkflow_depth == 0`, + `is_enabled`) and skips the first iteration (`limits.current_iteration == 0`). The save decision is `_periodic_checkpoint_due(now)` (`every_agent` OR `every_seconds` throttle; first save always fires). `_save_checkpoint_on_failure` and the periodic path share `_write_checkpoint(error, trigger)` (which best-effort-guards provider `get_session_ids()` so it never raises). The periodic save wraps write+emit+rotate; on any failure it calls `_record_periodic_checkpoint_failure()` which emits a **`checkpoint_save_failed`** event (consecutive-failure count; surfaced by `ConsoleEventSubscriber` + JSONL + dashboard) so a recovery-reliant user isn't silently left without checkpoints. After a save the engine calls `rotate_periodic_checkpoints`; at a terminal **non-resumable** outcome (clean completion via `run()`/`resume()`, or an explicit `status: failed` terminate) `_cleanup_run_periodic_checkpoints()` deletes the run's periodic checkpoints (an unexpected failure leaves them in place alongside the failure checkpoint). `conductor checkpoints` shows a `Trigger` column and `—` for periodic rows' error type. See `examples/periodic-checkpoints.yaml` and `docs/workflow-syntax.md` (Periodic Checkpoints section). - **Terminate steps** (`type: terminate`): explicit terminal step with `status` (`success` | `failed`), Jinja2 `reason`, and optional `output_template` (a `dict[str, str]` that replaces `workflow.output:` when set; each value is rendered then passed through `_maybe_parse_json` so `"true"` becomes `True`, `"42"` becomes `42`, JSON literals are parsed). Reaching a terminate step ends the workflow immediately (no routes evaluated after). `success` → CLI exit 0, dashboard ✅, `workflow_completed { termination_reason, terminated_by, is_explicit: true, status }`; runs `on_complete` hook. `failed` → CLI exit 1 (with rendered output JSON still printed to stdout for downstream tooling), dashboard ❌, raises `WorkflowTerminated` (subclass of `ExecutionError`), emits `workflow_failed { error_type: "WorkflowTerminated", is_explicit: true, status, output }`, runs `on_error` hook, and **does not** save an on-failure checkpoint (explicit terminations are intentionally non-resumable). Terminate steps cannot have `routes`, `tools`, `output`, `prompt`, `model`, etc.; cannot be used as parallel-group members or as a for_each inline agent (route to one from those groups' `routes:` instead). Inside a sub-workflow, a `status: failed` terminate is downgraded at the parent boundary to `SubworkflowTerminatedError` (also a subclass of `ExecutionError`) preserving the child's rendered `terminated_output` / `terminated_reason` / `terminated_by` as structured attributes — the parent treats it as a normal sub-workflow failure (its own `workflow_failed` does NOT inherit `is_explicit: true`). For more detail see `examples/terminate.yaml`, `docs/workflow-syntax.md` (Terminate Steps section), and `plugins/conductor/skills/conductor/references/authoring.md`. - **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`. +- **Connect to an existing Copilot runtime (Copilot)**: `runtime.provider.runtime_url` (Copilot-only) points the provider at an already-running `copilot --server` runtime instead of spawning a nested one — every agent/model reuses the server's single authenticated session. Optional `runtime_token` (`SecretStr`, redacted, requires `runtime_url`) is the connection secret. Both fields fall back to env vars (`COPILOT_PROVIDER_RUNTIME_URL` / `COPILOT_PROVIDER_RUNTIME_TOKEN`) which activate the connection on their own (zero-YAML path for orchestrators such as Agency). `has_external_runtime()` is a separate axis from `has_custom_routing()` (so it does NOT trigger the endpoint-provider resolver), but both keep `ProviderSettings` from collapsing to the bare-string serialization. Schema rejects: `runtime_url` combined with any custom-routing field (`base_url`/`api_key`/`bearer_token`/`type`/`wire_api`/`headers`/`azure`) — mutually exclusive since the runtime is the endpoint; `runtime_token` without `runtime_url`; empty `runtime_token`; either field when `name != "copilot"`. Provider layer: `_resolve_runtime_connection()` (YAML then env) and `_build_client()` (in `copilot.py`). See `examples/copilot-existing-runtime.yaml` and `docs/configuration.md` (Connecting to an Existing Copilot Runtime). - **Validator block** (`validator:` on a provider-backed agent, issue #220): semantic output validation with retry-once. After the primary agent completes, `WorkflowEngine._apply_validator` runs `OutputValidator` (`engine/validator.py`) — a second LLM call (synthetic agent via `provider.execute`, `tools=[]`, `{passed, issues}` output schema) grading the output against `validator.criteria`. Fields: `criteria` (required, non-empty), `model` (defaults to the agent's model), `max_retries` (`Field(1, ge=0, le=1)` — hard-capped at 1; `0` = report-only). On `passed: false` and `max_retries > 0`, the primary re-runs **once** via `executor.execute` with a `## Validation feedback` section (the issues) appended to `guidance_section`; the second output is final (no second validation loop). **Fail-open**: validator errors / unparseable responses → treated as pass with a logged warning. Wired into the main loop, parallel groups, and for-each loops (guarded by `agent.validator and not output.partial`; for-each passes a `usage_label` so the row matches `[]`). Emits `agent_validator_start` / `agent_validator_complete { passed, issues, errored, tokens, cost_usd }` / `agent_validation_failed { issues, will_retry }` through the per-agent `event_callback` (so for-each events carry `item_key`). Cost: the validation call and any discarded first attempt are recorded as a separate `" (validator)"` usage row (primary row = effective output). Rejected on `script` / `human_gate` / `workflow` / `wait` / `set` / `terminate` types. Frontend: event types in `web/frontend/src/types/events.ts`, store handlers + `NodeData.validator_*` fields in `web/frontend/src/stores/workflow-store.ts`, detail UI in `web/frontend/src/components/detail/ValidatorDetail.tsx` (rebuild with `make build-frontend`). See `examples/validator.yaml` and `docs/workflow-syntax.md` (Validator section). ### Debugging `--web-bg` failures diff --git a/CHANGELOG.md b/CHANGELOG.md index e359d6f0..c55c44b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Connect the Copilot provider to an existing runtime** — `runtime.provider` + gains a Copilot-only `runtime_url` (plus optional `runtime_token`) that points + Conductor at an already-running `copilot --server` runtime instead of spawning + its own nested one; every agent/model then reuses the server's single + authenticated session. Both fields also resolve from the namespaced + `COPILOT_PROVIDER_RUNTIME_URL` / `COPILOT_PROVIDER_RUNTIME_TOKEN` environment + variables, which activate the connection with no YAML — the zero-config path + for external orchestrators (e.g. Agency) that already own an authenticated + Copilot process. Mutually exclusive with custom endpoint routing. See + `examples/copilot-existing-runtime.yaml` and the "Connecting to an Existing + Copilot Runtime" section of `docs/configuration.md`. - **Provider-supplied model pricing** — cost reporting now resolves pricing via a new `AgentProvider.get_model_pricing` hook before falling back to the static table. Resolution order is workflow `cost.pricing` → provider hook → built-in diff --git a/docs/configuration.md b/docs/configuration.md index 5871bba8..29bee89f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -233,6 +233,83 @@ worrying about per-call drift. demonstrates the full pattern with both Ollama (active) and Azure OpenAI (commented variant). +### Connecting to an Existing Copilot Runtime + +By default the Copilot provider spawns its own nested `copilot` runtime +process (one per provider instance) to run agents. Instead, you can point +Conductor at an **already-running** Copilot runtime that was started in +server mode by some other process. Conductor then connects to that runtime +and reuses its single authenticated session for every agent — no nested +`copilot` process is spawned. + +This is the recommended way to run Conductor inside an external +orchestrator that already owns an authenticated Copilot process. The +canonical example is Microsoft **Agency**: it launches one authenticated +`copilot --server` and hands Conductor a connection handle, so all of +Conductor's agents/models are just new sessions on that shared server. +Authentication is handled once, at the server — Conductor never needs the +runtime's credentials. + +#### YAML + +```yaml +workflow: + runtime: + provider: + name: copilot + runtime_url: localhost:3000 # "port", "host:port", or a full URL + runtime_token: ${COPILOT_RUNTIME_TOKEN} # optional shared secret + default_model: gpt-4o +``` + +- `runtime_url` — where the running runtime is listening. Accepts a bare + `"port"`, `"host:port"`, or a full URL. +- `runtime_token` — the shared secret the runtime was started with, if + any. Stored as a `SecretStr` (redacted in events/checkpoints/dashboard); + prefer `${VAR}` interpolation. Requires `runtime_url`. + +#### Environment variables (zero-YAML) + +The connection also activates from environment variables alone, so an +orchestrator can enable it without editing the workflow YAML: + +| Field | Env var | +|---|---| +| `runtime_url` | `COPILOT_PROVIDER_RUNTIME_URL` | +| `runtime_token` | `COPILOT_PROVIDER_RUNTIME_TOKEN` | + +The YAML value takes precedence; the environment variable is used as a +fallback when the YAML field is unset. These variables are namespaced under `COPILOT_PROVIDER_*` on +purpose: unlike ambient `OPENAI_*` variables, they can safely activate the +connection because they are specific to this feature. + +```bash +# Agency (or any orchestrator) side, conceptually: +copilot --server --port 3000 & # one authenticated runtime +export COPILOT_PROVIDER_RUNTIME_URL=localhost:3000 +export COPILOT_PROVIDER_RUNTIME_TOKEN= +conductor run review.yaml # connects; spawns no nested runtime +``` + +#### Rules and notes + +- `runtime_url` is **mutually exclusive** with custom endpoint routing + (`base_url` / `api_key` / `bearer_token` / `type` / `wire_api` / + `headers` / `azure`) — the external runtime *is* the endpoint. The schema + rejects the combination at config load. +- `runtime_token` requires `runtime_url` (a token with nowhere to connect + is a misconfiguration) and, like the other secrets, may not be empty. +- Closing the provider does **not** terminate the external runtime — the + SDK only shuts down runtimes it spawned itself, so the orchestrator-owned + server keeps running. +- Runtime-spawn-only options (custom CLI path, injected env, etc.) do not + apply when connecting to an existing runtime. + +#### Example workflow + +[`examples/copilot-existing-runtime.yaml`](../examples/copilot-existing-runtime.yaml) +demonstrates connecting to an already-running Copilot runtime. + ## Common Configuration Options These options work with both providers: diff --git a/examples/copilot-existing-runtime.yaml b/examples/copilot-existing-runtime.yaml new file mode 100644 index 00000000..8b26da42 --- /dev/null +++ b/examples/copilot-existing-runtime.yaml @@ -0,0 +1,83 @@ +# Connect the Copilot provider to an already-running Copilot runtime +# +# By default conductor's Copilot provider spawns its own nested `copilot` +# runtime process to execute agents. This example instead points conductor +# at an EXISTING runtime that some other process started in server mode +# (e.g. `copilot --server --port 3000`). Conductor connects to that runtime +# via the SDK's `RuntimeConnection.for_uri(...)` and reuses its single +# authenticated session for every agent — no nested `copilot` process is +# spawned, and authentication is handled once, at the server. +# +# This is the recommended pattern for running conductor inside an external +# orchestrator that already owns an authenticated Copilot process (for +# example, Microsoft Agency launches one authenticated `copilot --server` +# and hands conductor a connection handle). +# +# Two ways to configure the connection: +# +# 1. YAML (below): set `runtime_url` (and optional `runtime_token`) under +# `runtime.provider`. +# +# 2. Environment variables (no YAML needed — ideal for orchestrators): +# +# export COPILOT_PROVIDER_RUNTIME_URL=localhost:3000 +# export COPILOT_PROVIDER_RUNTIME_TOKEN= +# conductor run examples/copilot-existing-runtime.yaml \ +# --input question="What is Python?" +# +# The YAML value takes precedence; the env var is a fallback. +# +# Notes: +# - `runtime_url` accepts a bare "port", "host:port", or a full URL. +# - `runtime_token` is the shared secret the runtime was started with (if +# any). It is stored as a SecretStr and redacted from events/checkpoints; +# prefer ${VAR} interpolation so the literal never lands in YAML. +# - `runtime_url` is mutually exclusive with custom endpoint routing +# (base_url / api_key / type / ...): the external runtime IS the endpoint. +# - Closing conductor does NOT terminate the external runtime — the SDK only +# shuts down runtimes it spawned itself. + +workflow: + name: copilot-existing-runtime + description: Connect the Copilot provider to an already-running runtime server + version: "1.0.0" + entry_point: answerer + + runtime: + provider: + name: copilot + # Where the already-running `copilot --server` is listening. + runtime_url: ${COPILOT_PROVIDER_RUNTIME_URL:-localhost:3000} + # Optional shared secret the server was started with. Uncomment and set + # $COPILOT_RUNTIME_TOKEN if your server requires a connection token + # (the SDK rejects an empty token, so keep this commented when unused): + # runtime_token: ${COPILOT_RUNTIME_TOKEN} + # Alternatively, set COPILOT_PROVIDER_RUNTIME_TOKEN in the environment. + + default_model: gpt-4o + + input: + question: + type: string + required: true + description: The question to answer + +agents: + - name: answerer + description: Answers the user's question via the shared runtime + 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 }}" diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index c9b4e0e6..62582925 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -2091,9 +2091,7 @@ def _build_client(self) -> Any: "Connecting to existing Copilot runtime at %s (no nested runtime spawned)", url, ) - return CopilotClient( - connection=RuntimeConnection.for_uri(url, connection_token=token) - ) + return CopilotClient(connection=RuntimeConnection.for_uri(url, connection_token=token)) def _fix_pipe_blocking_mode(self) -> None: """Clear O_NONBLOCK on the Copilot CLI subprocess pipes. diff --git a/tests/test_config/test_provider_settings.py b/tests/test_config/test_provider_settings.py index 5afc3473..56649950 100644 --- a/tests/test_config/test_provider_settings.py +++ b/tests/test_config/test_provider_settings.py @@ -287,3 +287,83 @@ def test_azure_activates_custom_routing(self) -> None: azure=AzureProviderOptions(api_version="2024-10-21"), ) assert s.has_custom_routing() + + +class TestExternalRuntimeConnection: + """``runtime_url`` / ``runtime_token`` connect to an existing runtime.""" + + def test_runtime_url_only(self) -> None: + s = ProviderSettings(name="copilot", runtime_url="localhost:3000") + assert s.runtime_url == "localhost:3000" + assert s.runtime_token is None + assert s.has_external_runtime() + # A runtime connection is a separate axis from endpoint routing. + assert not s.has_custom_routing() + + def test_runtime_url_and_token(self) -> None: + s = ProviderSettings.model_validate( + {"name": "copilot", "runtime_url": "host:9000", "runtime_token": "sekret"} + ) + assert s.runtime_url == "host:9000" + assert isinstance(s.runtime_token, SecretStr) + assert s.runtime_token.get_secret_value() == "sekret" + assert s.has_external_runtime() + + def test_runtime_fields_prevent_bare_string_collapse(self) -> None: + """A runtime connection must survive round-trip serialization (it would + be lost if the object collapsed to the bare ``"copilot"`` string).""" + s = ProviderSettings(name="copilot", runtime_url="localhost:3000") + dumped = s.model_dump() + assert isinstance(dumped, dict) + assert dumped["runtime_url"] == "localhost:3000" + + def test_runtime_token_redacted_in_json_dump(self) -> None: + s = ProviderSettings.model_validate( + {"name": "copilot", "runtime_url": "x:1", "runtime_token": "topsecret"} + ) + dumped = s.model_dump(mode="json") + assert dumped["runtime_token"] == "**********" + assert "topsecret" not in str(dumped) + + def test_runtime_url_rejected_for_non_copilot(self) -> None: + with pytest.raises(ValidationError, match="only supported when name='copilot'"): + ProviderSettings(name="claude", runtime_url="x:1") + + def test_runtime_token_requires_runtime_url(self) -> None: + with pytest.raises(ValidationError, match="'runtime_token' requires 'runtime_url'"): + ProviderSettings(name="copilot", runtime_token="tok") + + def test_empty_runtime_token_rejected(self) -> None: + with pytest.raises(ValidationError, match="'runtime_token' is empty"): + ProviderSettings.model_validate( + {"name": "copilot", "runtime_url": "x:1", "runtime_token": ""} + ) + + @pytest.mark.parametrize( + "field,value", + [ + ("base_url", "http://x/v1"), + ("api_key", "k"), + ("bearer_token", "t"), + ("type", "openai"), + ("wire_api", "completions"), + ("headers", {"X-Foo": "1"}), + ], + ) + def test_runtime_url_mutually_exclusive_with_custom_routing( + self, field: str, value: object + ) -> None: + # Anchor fields (base_url/api_key/bearer_token) trip the dedicated + # mutual-exclusion check; non-anchor routing fields (type/wire_api/ + # headers) trip the pre-existing "cannot stand alone" rule first. + # Either way the combination is rejected at config load. + with pytest.raises(ValidationError, match="cannot be combined with|cannot stand alone"): + ProviderSettings( # type: ignore[arg-type] + name="copilot", runtime_url="localhost:3000", **{field: value} + ) + + def test_runtime_url_with_anchor_field_reports_mutual_exclusion(self) -> None: + """An anchor routing field alongside ``runtime_url`` reports the + dedicated mutual-exclusion error (not the anchorless one).""" + with pytest.raises(ValidationError, match="cannot be combined with"): + ProviderSettings(name="copilot", runtime_url="localhost:3000", base_url="http://x/v1") diff --git a/tests/test_providers/test_copilot_provider_routing.py b/tests/test_providers/test_copilot_provider_routing.py index 90043e14..97de3522 100644 --- a/tests/test_providers/test_copilot_provider_routing.py +++ b/tests/test_providers/test_copilot_provider_routing.py @@ -523,3 +523,70 @@ async def fake_create_provider(**kwargs: Any) -> Any: # Claude call MUST NOT receive Copilot-shaped settings. assert by_type["claude"]["provider_settings"] is None + + +class TestResolveRuntimeConnection: + """Unit-tests for ``CopilotProvider._resolve_runtime_connection`` and + ``_build_client`` (connecting to an already-running Copilot runtime).""" + + def test_no_settings_no_env_returns_none(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_URL", raising=False) + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_TOKEN", raising=False) + provider = _make_provider() + assert provider._resolve_runtime_connection() is None + + def test_yaml_url_and_token(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_URL", raising=False) + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_TOKEN", raising=False) + s = ProviderSettings.model_validate( + {"name": "copilot", "runtime_url": "localhost:3000", "runtime_token": "sek"} + ) + provider = _make_provider(provider_settings=s) + assert provider._resolve_runtime_connection() == ("localhost:3000", "sek") + + def test_env_vars_alone_activate(self, monkeypatch: pytest.MonkeyPatch) -> None: + """The namespaced env vars activate the connection with no YAML — the + zero-config path for external orchestrators.""" + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_URL", "host:9000") + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_TOKEN", "envtok") + provider = _make_provider() + assert provider._resolve_runtime_connection() == ("host:9000", "envtok") + + def test_yaml_url_beats_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_URL", "env:1") + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_TOKEN", "envtok") + s = ProviderSettings(name="copilot", runtime_url="yaml:2") + provider = _make_provider(provider_settings=s) + # YAML url wins; token falls back to env since YAML has none. + assert provider._resolve_runtime_connection() == ("yaml:2", "envtok") + + def test_url_without_token(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_URL", "host:1") + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_TOKEN", raising=False) + provider = _make_provider() + assert provider._resolve_runtime_connection() == ("host:1", None) + + def test_build_client_default_spawns(self, monkeypatch: pytest.MonkeyPatch) -> None: + """No runtime connection → default ``CopilotClient()`` (spawns runtime).""" + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_URL", raising=False) + provider = _make_provider() + client = provider._build_client() + assert client._is_external_server is False + + def test_build_client_connects_to_external_runtime( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """With a runtime connection resolved, the client is a URI (external) + connection and does not spawn a nested runtime.""" + from copilot.client import UriRuntimeConnection + + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_URL", raising=False) + s = ProviderSettings.model_validate( + {"name": "copilot", "runtime_url": "localhost:3000", "runtime_token": "sek"} + ) + provider = _make_provider(provider_settings=s) + client = provider._build_client() + assert client._is_external_server is True + assert isinstance(client._connection, UriRuntimeConnection) + assert client._connection.url == "localhost:3000" + assert client._connection.connection_token == "sek" From be2d2db0b5c0c24b22b2420e3c7d417565704f0f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:22:23 +0000 Subject: [PATCH 3/4] Remove 'Agency' references from public SDK docs and comments --- AGENTS.md | 4 ++-- CHANGELOG.md | 2 +- docs/configuration.md | 8 ++++---- examples/copilot-existing-runtime.yaml | 2 +- src/conductor/config/schema.py | 2 +- src/conductor/providers/copilot.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2f14c71b..9afe186f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,7 +111,7 @@ step-by-step checklist. - **providers/**: SDK provider abstraction - `base.py` - `AgentProvider` ABC defining `execute()`, `validate_connection()`, `close()` - - `copilot.py` - GitHub Copilot SDK implementation. By default spawns a nested `copilot` runtime via `CopilotClient()` (in `_build_client`, called from `_ensure_client_started`). When a runtime connection is resolved (`runtime.provider.runtime_url` or `COPILOT_PROVIDER_RUNTIME_URL`, optional `runtime_token` / `COPILOT_PROVIDER_RUNTIME_TOKEN`), it instead builds `CopilotClient(connection=RuntimeConnection.for_uri(url, connection_token=token))` to connect to an already-running `copilot --server`; the SDK skips spawning for URI connections and its `stop()` leaves the externally-owned server running. `_resolve_runtime_connection()` reads YAML first, then the namespaced env var (env activates on its own — the zero-YAML path for orchestrators like Agency). Mutually exclusive with custom endpoint routing. + - `copilot.py` - GitHub Copilot SDK implementation. By default spawns a nested `copilot` runtime via `CopilotClient()` (in `_build_client`, called from `_ensure_client_started`). When a runtime connection is resolved (`runtime.provider.runtime_url` or `COPILOT_PROVIDER_RUNTIME_URL`, optional `runtime_token` / `COPILOT_PROVIDER_RUNTIME_TOKEN`), it instead builds `CopilotClient(connection=RuntimeConnection.for_uri(url, connection_token=token))` to connect to an already-running `copilot --server`; the SDK skips spawning for URI connections and its `stop()` leaves the externally-owned server running. `_resolve_runtime_connection()` reads YAML first, then the namespaced env var (env activates on its own — the zero-YAML path for external orchestrators). Mutually exclusive with custom endpoint routing. - `claude.py` - Anthropic Claude API implementation - `claude_agent_sdk.py` - Claude Agent SDK implementation (uses `claude-agent-sdk` package) - `factory.py` - Provider instantiation @@ -154,7 +154,7 @@ step-by-step checklist. - **Periodic checkpoints** (`runtime.checkpoint`, issue #244): opt-in `CheckpointConfig` (`every_agent: bool`, `every_seconds: int|None`, `keep_last: int=5`; `is_enabled = every_agent or every_seconds is not None`). Off by default → failure-only behavior preserved. `WorkflowEngine._maybe_save_periodic_checkpoint()` is called once at the **top of `_execute_loop`** (single choke point), where prior outputs are committed and `_current_agent_name` is the step *about to run* — so a periodic checkpoint reuses failure-checkpoint `current_agent` semantics and resume continues forward with no special-casing. Gated via the `_periodic_checkpoints_active` property (**root engine only**, `_subworkflow_depth == 0`, + `is_enabled`) and skips the first iteration (`limits.current_iteration == 0`). The save decision is `_periodic_checkpoint_due(now)` (`every_agent` OR `every_seconds` throttle; first save always fires). `_save_checkpoint_on_failure` and the periodic path share `_write_checkpoint(error, trigger)` (which best-effort-guards provider `get_session_ids()` so it never raises). The periodic save wraps write+emit+rotate; on any failure it calls `_record_periodic_checkpoint_failure()` which emits a **`checkpoint_save_failed`** event (consecutive-failure count; surfaced by `ConsoleEventSubscriber` + JSONL + dashboard) so a recovery-reliant user isn't silently left without checkpoints. After a save the engine calls `rotate_periodic_checkpoints`; at a terminal **non-resumable** outcome (clean completion via `run()`/`resume()`, or an explicit `status: failed` terminate) `_cleanup_run_periodic_checkpoints()` deletes the run's periodic checkpoints (an unexpected failure leaves them in place alongside the failure checkpoint). `conductor checkpoints` shows a `Trigger` column and `—` for periodic rows' error type. See `examples/periodic-checkpoints.yaml` and `docs/workflow-syntax.md` (Periodic Checkpoints section). - **Terminate steps** (`type: terminate`): explicit terminal step with `status` (`success` | `failed`), Jinja2 `reason`, and optional `output_template` (a `dict[str, str]` that replaces `workflow.output:` when set; each value is rendered then passed through `_maybe_parse_json` so `"true"` becomes `True`, `"42"` becomes `42`, JSON literals are parsed). Reaching a terminate step ends the workflow immediately (no routes evaluated after). `success` → CLI exit 0, dashboard ✅, `workflow_completed { termination_reason, terminated_by, is_explicit: true, status }`; runs `on_complete` hook. `failed` → CLI exit 1 (with rendered output JSON still printed to stdout for downstream tooling), dashboard ❌, raises `WorkflowTerminated` (subclass of `ExecutionError`), emits `workflow_failed { error_type: "WorkflowTerminated", is_explicit: true, status, output }`, runs `on_error` hook, and **does not** save an on-failure checkpoint (explicit terminations are intentionally non-resumable). Terminate steps cannot have `routes`, `tools`, `output`, `prompt`, `model`, etc.; cannot be used as parallel-group members or as a for_each inline agent (route to one from those groups' `routes:` instead). Inside a sub-workflow, a `status: failed` terminate is downgraded at the parent boundary to `SubworkflowTerminatedError` (also a subclass of `ExecutionError`) preserving the child's rendered `terminated_output` / `terminated_reason` / `terminated_by` as structured attributes — the parent treats it as a normal sub-workflow failure (its own `workflow_failed` does NOT inherit `is_explicit: true`). For more detail see `examples/terminate.yaml`, `docs/workflow-syntax.md` (Terminate Steps section), and `plugins/conductor/skills/conductor/references/authoring.md`. - **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`. -- **Connect to an existing Copilot runtime (Copilot)**: `runtime.provider.runtime_url` (Copilot-only) points the provider at an already-running `copilot --server` runtime instead of spawning a nested one — every agent/model reuses the server's single authenticated session. Optional `runtime_token` (`SecretStr`, redacted, requires `runtime_url`) is the connection secret. Both fields fall back to env vars (`COPILOT_PROVIDER_RUNTIME_URL` / `COPILOT_PROVIDER_RUNTIME_TOKEN`) which activate the connection on their own (zero-YAML path for orchestrators such as Agency). `has_external_runtime()` is a separate axis from `has_custom_routing()` (so it does NOT trigger the endpoint-provider resolver), but both keep `ProviderSettings` from collapsing to the bare-string serialization. Schema rejects: `runtime_url` combined with any custom-routing field (`base_url`/`api_key`/`bearer_token`/`type`/`wire_api`/`headers`/`azure`) — mutually exclusive since the runtime is the endpoint; `runtime_token` without `runtime_url`; empty `runtime_token`; either field when `name != "copilot"`. Provider layer: `_resolve_runtime_connection()` (YAML then env) and `_build_client()` (in `copilot.py`). See `examples/copilot-existing-runtime.yaml` and `docs/configuration.md` (Connecting to an Existing Copilot Runtime). +- **Connect to an existing Copilot runtime (Copilot)**: `runtime.provider.runtime_url` (Copilot-only) points the provider at an already-running `copilot --server` runtime instead of spawning a nested one — every agent/model reuses the server's single authenticated session. Optional `runtime_token` (`SecretStr`, redacted, requires `runtime_url`) is the connection secret. Both fields fall back to env vars (`COPILOT_PROVIDER_RUNTIME_URL` / `COPILOT_PROVIDER_RUNTIME_TOKEN`) which activate the connection on their own (zero-YAML path for external orchestrators). `has_external_runtime()` is a separate axis from `has_custom_routing()` (so it does NOT trigger the endpoint-provider resolver), but both keep `ProviderSettings` from collapsing to the bare-string serialization. Schema rejects: `runtime_url` combined with any custom-routing field (`base_url`/`api_key`/`bearer_token`/`type`/`wire_api`/`headers`/`azure`) — mutually exclusive since the runtime is the endpoint; `runtime_token` without `runtime_url`; empty `runtime_token`; either field when `name != "copilot"`. Provider layer: `_resolve_runtime_connection()` (YAML then env) and `_build_client()` (in `copilot.py`). See `examples/copilot-existing-runtime.yaml` and `docs/configuration.md` (Connecting to an Existing Copilot Runtime). - **Validator block** (`validator:` on a provider-backed agent, issue #220): semantic output validation with retry-once. After the primary agent completes, `WorkflowEngine._apply_validator` runs `OutputValidator` (`engine/validator.py`) — a second LLM call (synthetic agent via `provider.execute`, `tools=[]`, `{passed, issues}` output schema) grading the output against `validator.criteria`. Fields: `criteria` (required, non-empty), `model` (defaults to the agent's model), `max_retries` (`Field(1, ge=0, le=1)` — hard-capped at 1; `0` = report-only). On `passed: false` and `max_retries > 0`, the primary re-runs **once** via `executor.execute` with a `## Validation feedback` section (the issues) appended to `guidance_section`; the second output is final (no second validation loop). **Fail-open**: validator errors / unparseable responses → treated as pass with a logged warning. Wired into the main loop, parallel groups, and for-each loops (guarded by `agent.validator and not output.partial`; for-each passes a `usage_label` so the row matches `[]`). Emits `agent_validator_start` / `agent_validator_complete { passed, issues, errored, tokens, cost_usd }` / `agent_validation_failed { issues, will_retry }` through the per-agent `event_callback` (so for-each events carry `item_key`). Cost: the validation call and any discarded first attempt are recorded as a separate `" (validator)"` usage row (primary row = effective output). Rejected on `script` / `human_gate` / `workflow` / `wait` / `set` / `terminate` types. Frontend: event types in `web/frontend/src/types/events.ts`, store handlers + `NodeData.validator_*` fields in `web/frontend/src/stores/workflow-store.ts`, detail UI in `web/frontend/src/components/detail/ValidatorDetail.tsx` (rebuild with `make build-frontend`). See `examples/validator.yaml` and `docs/workflow-syntax.md` (Validator section). ### Debugging `--web-bg` failures diff --git a/CHANGELOG.md b/CHANGELOG.md index c55c44b9..5c66ef3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 authenticated session. Both fields also resolve from the namespaced `COPILOT_PROVIDER_RUNTIME_URL` / `COPILOT_PROVIDER_RUNTIME_TOKEN` environment variables, which activate the connection with no YAML — the zero-config path - for external orchestrators (e.g. Agency) that already own an authenticated + for external orchestrators that already own an authenticated Copilot process. Mutually exclusive with custom endpoint routing. See `examples/copilot-existing-runtime.yaml` and the "Connecting to an Existing Copilot Runtime" section of `docs/configuration.md`. diff --git a/docs/configuration.md b/docs/configuration.md index 29bee89f..626bb78e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -243,9 +243,9 @@ and reuses its single authenticated session for every agent — no nested `copilot` process is spawned. This is the recommended way to run Conductor inside an external -orchestrator that already owns an authenticated Copilot process. The -canonical example is Microsoft **Agency**: it launches one authenticated -`copilot --server` and hands Conductor a connection handle, so all of +orchestrator that already owns an authenticated Copilot process. For +example, an orchestrator can launch one authenticated +`copilot --server` and hand Conductor a connection handle, so all of Conductor's agents/models are just new sessions on that shared server. Authentication is handled once, at the server — Conductor never needs the runtime's credentials. @@ -284,7 +284,7 @@ purpose: unlike ambient `OPENAI_*` variables, they can safely activate the connection because they are specific to this feature. ```bash -# Agency (or any orchestrator) side, conceptually: +# Orchestrator side, conceptually: copilot --server --port 3000 & # one authenticated runtime export COPILOT_PROVIDER_RUNTIME_URL=localhost:3000 export COPILOT_PROVIDER_RUNTIME_TOKEN= diff --git a/examples/copilot-existing-runtime.yaml b/examples/copilot-existing-runtime.yaml index 8b26da42..394763f5 100644 --- a/examples/copilot-existing-runtime.yaml +++ b/examples/copilot-existing-runtime.yaml @@ -10,7 +10,7 @@ # # This is the recommended pattern for running conductor inside an external # orchestrator that already owns an authenticated Copilot process (for -# example, Microsoft Agency launches one authenticated `copilot --server` +# example, an orchestrator launches one authenticated `copilot --server` # and hands conductor a connection handle). # # Two ways to configure the connection: diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index cc091fcc..68091abb 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -1720,7 +1720,7 @@ class ProviderSettings(BaseModel): ``RuntimeConnection.for_uri(...)`` — no child ``copilot`` process is spawned, so every agent/model reuses the server's single authenticated session. This is the recommended way to run Conductor inside an external - orchestrator (e.g. Agency) that already owns an authenticated + orchestrator that already owns an authenticated ``copilot --server`` process. Falls back to the ``COPILOT_PROVIDER_RUNTIME_URL`` environment variable diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index 62582925..01f023d6 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -476,7 +476,7 @@ def _resolve_runtime_connection(self) -> tuple[str, str | None] | None: The environment variables activate the connection on their own (no YAML required), which is the intended zero-config path for external - orchestrators such as Agency: they launch one authenticated + orchestrators: they launch one authenticated ``copilot --server`` and export these two variables. The variables are namespaced (``COPILOT_PROVIDER_*``) specifically so unrelated ambient shell state cannot silently divert default Copilot traffic. From 6895e710af579faa5b0701385841ca7edd206379 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Thu, 9 Jul 2026 21:49:11 +0100 Subject: [PATCH 4/4] Repair PR #283: address review comments (R1-001, R1-002, R2-001, R2-002) - R1-001: import RuntimeConnection in its own try/except so an older-but-present SDK still enables the default Copilot provider - R1-002: reject empty runtime_url at schema validation - R2-001: raise ProviderError (not warn) when an external runtime is combined with custom endpoint routing - R2-002: normalize/validate env-resolved runtime_url/runtime_token (empty URL raises, token-only raises, empty token -> None) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/conductor/config/schema.py | 10 ++++ src/conductor/providers/copilot.py | 58 +++++++++++++++---- tests/test_config/test_provider_settings.py | 14 +++++ .../test_copilot_provider_routing.py | 44 ++++++++++++++ 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/src/conductor/config/schema.py b/src/conductor/config/schema.py index 68091abb..4bfa4bc4 100644 --- a/src/conductor/config/schema.py +++ b/src/conductor/config/schema.py @@ -1850,6 +1850,16 @@ def _check_field_compatibility(self) -> ProviderSettings: "(typo / unset env interpolation?)" ) + # An empty runtime_url must also be rejected: because "" is not None, + # has_external_runtime() would return True and the runtime_token guard + # (runtime_url is None) would pass, yet the provider treats "" as falsy + # and silently falls back to env / a nested spawn while dropping the token. + if self.runtime_url is not None and self.runtime_url == "": + raise ValueError( + "'runtime_url' 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 diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index 01f023d6..ab813660 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -79,16 +79,23 @@ def _is_finite_nonneg(value: object) -> TypeGuard[float]: # Try to import the Copilot SDK try: from copilot import CopilotClient - from copilot.client import RuntimeConnection from copilot.session import PermissionHandler COPILOT_SDK_AVAILABLE = True except ImportError: COPILOT_SDK_AVAILABLE = False CopilotClient = None # type: ignore[misc, assignment] - RuntimeConnection = None # type: ignore[misc, assignment] PermissionHandler = None # type: ignore[misc, assignment] +# RuntimeConnection was added to the SDK after CopilotClient; import it +# separately so an older-but-present SDK still enables the default nested-spawn +# provider. A runtime connection is only required when explicitly requested, +# and that path raises a clear ProviderError if RuntimeConnection is missing. +try: + from copilot.client import RuntimeConnection +except ImportError: + RuntimeConnection = None # type: ignore[misc, assignment] + @dataclass class RetryConfig: @@ -492,9 +499,37 @@ def _resolve_runtime_connection(self) -> tuple[str, str | None] | None: if token is None: token = os.environ.get("COPILOT_PROVIDER_RUNTIME_TOKEN") - if not url: + # Values resolved from environment variables are not validated by the + # schema, so normalize/validate them here to mirror the YAML rules and + # avoid silently falling through to a nested spawn on a typo/unset env. + if url is not None: + url = url.strip() + if not url: + raise ProviderError( + "'runtime_url' is empty; remove it or supply a value.", + suggestion=( + "Set runtime.provider.runtime_url or COPILOT_PROVIDER_RUNTIME_URL " + "to a valid port, host:port, or full URL." + ), + is_retryable=False, + ) + # An empty token is the legitimate no-auth / tokenless-runtime case; + # normalize it to None rather than erroring. + if token is not None: + token = token.strip() or None + if token is not None and url is None: + raise ProviderError( + "'runtime_token' requires 'runtime_url' to also be set", + suggestion=( + "Set COPILOT_PROVIDER_RUNTIME_URL alongside " + "COPILOT_PROVIDER_RUNTIME_TOKEN, or remove the token." + ), + is_retryable=False, + ) + + if url is None: return None - return (url, token or None) + return (url, token) async def execute( self, @@ -2080,12 +2115,15 @@ def _build_client(self) -> Any: url, token = connection if self._provider_settings is not None and self._provider_settings.has_custom_routing(): - logger.warning( - "Both an external runtime connection (runtime_url=%s) and custom endpoint " - "routing are configured; the custom endpoint routing will still be applied " - "to sessions on the external runtime. These are usually mutually exclusive — " - "verify this is intended.", - url, + raise ProviderError( + "An external runtime connection (runtime_url) cannot be combined with custom " + "endpoint routing in runtime.provider; connecting to an existing runtime and " + "custom endpoint routing are mutually exclusive (the runtime is the endpoint).", + suggestion=( + "Unset COPILOT_PROVIDER_RUNTIME_URL or remove base_url/api_key/" + "bearer_token/type/wire_api/headers/azure from runtime.provider." + ), + is_retryable=False, ) logger.info( "Connecting to existing Copilot runtime at %s (no nested runtime spawned)", diff --git a/tests/test_config/test_provider_settings.py b/tests/test_config/test_provider_settings.py index 56649950..84637f89 100644 --- a/tests/test_config/test_provider_settings.py +++ b/tests/test_config/test_provider_settings.py @@ -339,6 +339,20 @@ def test_empty_runtime_token_rejected(self) -> None: {"name": "copilot", "runtime_url": "x:1", "runtime_token": ""} ) + def test_empty_runtime_url_rejected(self) -> None: + """An empty ``runtime_url`` must be rejected: otherwise ``""`` is not + None, so ``has_external_runtime()`` returns True and the runtime_token + guard passes, yet the provider treats ``""`` as falsy and silently + spawns a nested runtime while dropping the token.""" + with pytest.raises(ValidationError, match="'runtime_url' is empty"): + ProviderSettings.model_validate({"name": "copilot", "runtime_url": ""}) + + def test_empty_runtime_url_with_token_rejected(self) -> None: + with pytest.raises(ValidationError, match="'runtime_url' is empty"): + ProviderSettings.model_validate( + {"name": "copilot", "runtime_url": "", "runtime_token": "tok"} + ) + @pytest.mark.parametrize( "field,value", [ diff --git a/tests/test_providers/test_copilot_provider_routing.py b/tests/test_providers/test_copilot_provider_routing.py index 97de3522..83658452 100644 --- a/tests/test_providers/test_copilot_provider_routing.py +++ b/tests/test_providers/test_copilot_provider_routing.py @@ -18,6 +18,7 @@ ProviderSettings, RuntimeConfig, ) +from conductor.exceptions import ProviderError from conductor.providers.copilot import CopilotProvider @@ -590,3 +591,46 @@ def test_build_client_connects_to_external_runtime( assert isinstance(client._connection, UriRuntimeConnection) assert client._connection.url == "localhost:3000" assert client._connection.connection_token == "sek" + + def test_empty_env_url_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + """An empty ``COPILOT_PROVIDER_RUNTIME_URL`` (e.g. ``${VAR:-}``) must fail + loudly rather than silently falling through to a nested spawn.""" + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_URL", " ") + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_TOKEN", raising=False) + provider = _make_provider() + with pytest.raises(ProviderError, match="runtime_url' is empty"): + provider._resolve_runtime_connection() + + def test_env_token_only_raises(self, monkeypatch: pytest.MonkeyPatch) -> None: + """A token with no URL must raise, mirroring the YAML rule.""" + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_URL", raising=False) + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_TOKEN", "envtok") + provider = _make_provider() + with pytest.raises(ProviderError, match="runtime_token' requires 'runtime_url'"): + provider._resolve_runtime_connection() + + def test_empty_env_token_normalizes_to_none(self, monkeypatch: pytest.MonkeyPatch) -> None: + """An empty token is the legitimate no-auth case → normalize to None.""" + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_URL", "host:1") + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_TOKEN", " ") + provider = _make_provider() + assert provider._resolve_runtime_connection() == ("host:1", None) + + def test_build_client_rejects_runtime_plus_custom_routing( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An env-supplied runtime URL combined with YAML custom endpoint routing + must fail fast rather than silently connecting to the external runtime.""" + monkeypatch.setenv("COPILOT_PROVIDER_RUNTIME_URL", "host:9000") + monkeypatch.delenv("COPILOT_PROVIDER_RUNTIME_TOKEN", raising=False) + s = ProviderSettings.model_validate( + { + "name": "copilot", + "type": "openai", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-yaml", + } + ) + provider = _make_provider(provider_settings=s, model="ollama/llama3") + with pytest.raises(ProviderError, match="mutually exclusive"): + provider._build_client()