fix: wire BYOK provider into headless sidecar env so sub-agents inherit it#42613
Conversation
…R_* env vars Sub-agents spawned by the Copilot CLI task tool do not inherit the SDK session-level `provider` config. Instead, the headless sidecar reads COPILOT_PROVIDER_* from its process env to configure each sub-agent session's inference backend. Forward the resolved BYOK provider config as native COPILOT_PROVIDER_BASE_URL and COPILOT_PROVIDER_TYPE env vars in sdkChildEnv so the headless sidecar uses the same AWF proxy for all sessions including sub-agents. This fixes the '400 no model endpoints available given user constraints' error that occurs when pr-processor sub-agents try to use CAPI directly for a model only accessible through the AWF proxy. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #42613 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
There was a problem hiding this comment.
Pull request overview
This pull request fixes Copilot SDK BYOK mode failures for sub-agents spawned via the task tool by ensuring the headless Copilot sidecar process receives the same BYOK provider configuration as the parent session. It forwards the resolved BYOK provider settings into the sidecar’s process environment using the native COPILOT_PROVIDER_* variables, so all sessions the sidecar creates (including sub-agent sessions) consistently use the intended inference backend.
Changes:
- Forward BYOK provider configuration into the SDK child environment as native
COPILOT_PROVIDER_BASE_URLandCOPILOT_PROVIDER_TYPE. - Conditionally forward
COPILOT_PROVIDER_WIRE_APIwhenwireApiis present, matching existing “optional” semantics. - Add in-file documentation clarifying why the extra
COPILOT_PROVIDER_*forwarding is required for sub-agent propagation in SDK mode.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/copilot_harness.cjs |
Adds native COPILOT_PROVIDER_* env vars to the SDK-mode child env so the headless sidecar (and sub-agent sessions it spawns) inherit the resolved BYOK provider configuration. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Low
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ APPROVE
The fix is correct and minimal. COPILOT_PROVIDER_* vars are injected only in the copilotSDKMode branch, where BYOK resolution is mandatory (or the process exits at line 727). Values are therefore always valid and non-empty.
Analysis notes
- Conditional
COPILOT_PROVIDER_WIRE_API: Using a conditional spread instead of always setting an empty string is the right pattern — avoids polluting the sidecar env with a meaningless empty-string var for an optional feature. providerBaseUrlalways non-empty in SDK mode: Theprocess.exit(1)guard ensures BYOK resolution succeeds before these vars are injected, soCOPILOT_PROVIDER_BASE_URLwill never be empty in practice.childEnvoverride ordering:{ ...process.env, ...sdkChildEnv }correctly ensures the resolved BYOK values take precedence over any pre-existingCOPILOT_PROVIDER_*in the parent environment.- No race conditions, no security concerns, no logic errors in the changed lines.
🔎 Code quality review by PR Code Quality Reviewer · 32.6 AIC · ⌖ 9.41 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs, /tdd, and /codebase-design — 2 observations, no blockers.
📋 Key Themes & Highlights
Key Themes
- Missing regression test: The fix addresses a concrete error (
400 no model endpoints available) but there is no test to guard against this regressing silently. BYOK + sub-agent paths are hard to cover end-to-end, but even a unit-level check onsdkChildEnvconstruction would add safety. - Wire-API var inconsistency:
GH_AW_COPILOT_SDK_PROVIDER_WIRE_APIis always emitted (even as""), but the newCOPILOT_PROVIDER_WIRE_APIfollows a cleaner conditional-omit pattern. Worth aligning the old var to the new convention.
Positive Highlights
- ✅ Root cause clearly diagnosed and explained in the PR description
- ✅ Fix is surgical — exactly the right minimal change, no unrelated churn
- ✅ Conditional spread for
COPILOT_PROVIDER_WIRE_APIis the correct pattern (omit rather than empty-string) - ✅ Comment block above the env object is thorough and will help future maintainers
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 54 AIC · ⌖ 7.48 AIC · ⊞ 6.6K
Comment /matt to run again
| // Native Copilot CLI BYOK env vars — consumed by the headless sidecar for all sessions. | ||
| COPILOT_PROVIDER_BASE_URL: providerBaseUrl, | ||
| COPILOT_PROVIDER_TYPE: providerType, | ||
| ...(providerWireApi ? { COPILOT_PROVIDER_WIRE_API: providerWireApi } : {}), |
There was a problem hiding this comment.
[/tdd] No regression test accompanies this bug fix — if COPILOT_PROVIDER_* forwarding regresses, the 400 no model endpoints available failure will be silent until manual BYOK testing catches it.
💡 Suggested test skeleton
A unit test that constructs sdkChildEnv with a mocked customProvider and asserts all three COPILOT_PROVIDER_* keys are present would catch future regressions:
it("sdkChildEnv includes COPILOT_PROVIDER_* vars when BYOK is configured", () => {
const env = buildSdkChildEnv({ providerBaseUrl: "(api.example.com/redacted)", providerType: "openai", providerWireApi: "v1" });
expect(env.COPILOT_PROVIDER_BASE_URL).toBe("(api.example.com/redacted)");
expect(env.COPILOT_PROVIDER_TYPE).toBe("openai");
expect(env.COPILOT_PROVIDER_WIRE_API).toBe("v1");
});
it("omits COPILOT_PROVIDER_WIRE_API when wireApi is empty", () => {
const env = buildSdkChildEnv({ providerBaseUrl: "(api.example.com/redacted)", providerType: "openai", providerWireApi: "" });
expect(env).not.toHaveProperty("COPILOT_PROVIDER_WIRE_API");
});If extracting buildSdkChildEnv is too invasive, a lighter option is an integration smoke-test that checks the spawned child process receives the expected env vars.
@copilot please address this.
| @@ -750,6 +756,10 @@ async function main() { | |||
| GH_AW_COPILOT_SDK_PROVIDER_TYPE: providerType, | |||
| GH_AW_COPILOT_SDK_PROVIDER_WIRE_API: providerWireApi, | |||
There was a problem hiding this comment.
[/codebase-design] GH_AW_COPILOT_SDK_PROVIDER_WIRE_API is always set (possibly to "") while the parallel new var COPILOT_PROVIDER_WIRE_API is omitted when empty — the two sibling consumers see different shapes of the same absence.
💡 Suggestion
For consistent semantics, consider aligning the existing var with the new conditional pattern:
// Before (always sets, even empty string)
GH_AW_COPILOT_SDK_PROVIDER_WIRE_API: providerWireApi,
// After (omit when absent, matching COPILOT_PROVIDER_WIRE_API)
...(providerWireApi ? { GH_AW_COPILOT_SDK_PROVIDER_WIRE_API: providerWireApi } : {}),If the driver treats "" and undefined identically this is low-risk cosmetic. If it distinguishes them, the inconsistency is a latent bug.
@copilot please address this.
There was a problem hiding this comment.
Review: BYOK provider propagation to sub-agent sessions
The change is correct and well-reasoned. In SDK/BYOK mode, the headless sidecar spawns sub-agent sessions using its own process env — not the parent session SDK config — so forwarding COPILOT_PROVIDER_* native vars is the right fix.
What was checked:
providerBaseUrl/providerTypeare always resolved before this point (guarded byprocess.exit(1)on BYOK resolution failure), so no empty-value risk.- The conditional spread for
COPILOT_PROVIDER_WIRE_APIis correct: avoids setting an empty env var that the sidecar might misinterpret (more careful than the pre-existingGH_AW_COPILOT_SDK_PROVIDER_WIRE_API: providerWireApiwhich passes empty string unconditionally). - Change is scoped entirely to
copilotSDKMode, no impact on non-SDK paths. - No secrets or credentials are propagated — only endpoint URLs and type strings.
✅ Minimal, targeted, and matches the stated intent.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 36.7 AIC · ⌖ 6.92 AIC · ⊞ 4.9K
|
@copilot run pr-finisher skill |
|
✅ PR Branch Updated The branch has been updated with the latest changes from @copilot Please run the
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
In copilot-sdk BYOK mode, sub-agents spawned via the
tasktool fail with400 no model endpoints available given user constraintsbecause they don't inherit the SDK session-levelproviderconfig. The headless sidecar configures each sub-agent session from its own process env (COPILOT_PROVIDER_*), not from the parent session's SDK config.Changes
actions/setup/js/copilot_harness.cjs— AddCOPILOT_PROVIDER_BASE_URL,COPILOT_PROVIDER_TYPE, and (when applicable)COPILOT_PROVIDER_WIRE_APItosdkChildEnvalongside the existingGH_AW_COPILOT_SDK_PROVIDER_*vars:The sidecar now uses the AWF proxy for every session it creates — main session and sub-agents alike.