Skip to content

feat(copilot): add BYOK provider parity#96345

Merged
vincentkoc merged 1 commit into
mainfrom
harness-parity-audit
Jun 24, 2026
Merged

feat(copilot): add BYOK provider parity#96345
vincentkoc merged 1 commit into
mainfrom
harness-parity-audit

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jun 24, 2026

Copy link
Copy Markdown
Member

What Problem This Solves

Copilot was still behind the Codex harness path for BYOK and runtime-session parity: custom provider routing could not safely use the Copilot SDK session-level provider contract, native compaction did not carry the resolved runtime model/API key consistently, and harness selection could not distinguish unowned custom providers from provider ids owned by another plugin/runtime.

Why This Change Was Made

  • Adds a generic provider-ownership signal for harness selection so plugin harnesses can claim only the custom provider ids they can actually own.
  • Adds a plugin-SDK request transport seam and native compaction runtime-model plumbing so BYOK/native paths share the same resolved model/auth contract.
  • Adds Copilot BYOK provider mapping and a loopback proxy that keeps SDK-local traffic local while outbound provider traffic stays behind OpenClaw guarded fetch.
  • Keeps unsafe transport shapes out of Copilot BYOK: no provider proxy/TLS/private-network overrides, HTTPS-only provider endpoints, no URL userinfo, no credential-bearing query params, SSRF-blocked hosts rejected, and guarded fetch enforces HTTPS across redirects.
  • Updates the plugin SDK surface budget for the new public harness runtime helpers.

User Impact

Copilot can now run explicit agentRuntime.id: "copilot" custom BYOK provider entries for supported OpenAI-compatible, Azure OpenAI Responses, Ollama-compatible completions, and Anthropic Messages shapes. Native provider ids such as openai, anthropic, google, and ollama remain owned by their native runtimes; Copilot only claims unowned custom provider ids.

Compaction and session reuse are keyed against the actual runtime model, endpoint, headers, credential fingerprint, and token limits, so rotated BYOK configuration gets a fresh Copilot SDK session instead of reusing an incompatible one.

Evidence

  • Live Copilot smoke: OPENCLAW_LIVE_TEST=1 OPENCLAW_COPILOT_AGENT_LIVE_TEST=1 ... attempt.live.test.ts passed earlier in this branch with the local GitHub token path.
  • Focused Testbox-through-Crabbox proof: tbx_01kvwhn813hxy2q7b3ef3fkfvq, GitHub Actions run 28091101470, passed node scripts/run-vitest.mjs extensions/copilot/src/provider-bridge.test.ts extensions/copilot/src/byok-proxy.test.ts extensions/copilot/harness.test.ts src/agents/harness/selection.test.ts src/plugins/providers.test.ts src/plugin-sdk/agent-harness-runtime.test.ts test/scripts/plugin-sdk-surface-report.test.ts.
  • Same Testbox run passed env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 corepack pnpm check:changed.
  • Structured autoreview on 76595ab5f9 returned clean: no accepted/actionable findings.
  • scripts/pr prepare-run 96345 reached full local pnpm test; broad local suite exposed unrelated baseline failures plus the now-fixed plugin SDK surface-budget drift. The focused surface test above validates the PR-owned fix.

@vincentkoc vincentkoc self-assigned this Jun 24, 2026
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling extensions: copilot size: XL maintainer Maintainer-authored PR labels Jun 24, 2026
@vincentkoc vincentkoc marked this pull request as ready for review June 24, 2026 08:49
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 5:09 AM ET / 09:09 UTC.

Summary
The PR adds Copilot BYOK provider parity through custom provider mapping, guarded loopback proxying, harness support context, session binding updates, public SDK helper exports, docs, and tests.

PR surface: Source +1162, Tests +1434, Docs +59, Generated 0. Total +2655 across 26 files.

Reproducibility: not applicable. this is a PR for a new Copilot BYOK capability rather than a bug report with a current-main reproduction path.

Review metrics: 3 noteworthy metrics.

  • Patch size: +2795/-140 across 26 files. This is an XL provider/runtime feature touching multiple compatibility-sensitive surfaces, so review should not be diff-snippet-only.
  • Plugin SDK surface: 1 existing public subpath export expanded. Public SDK exports are third-party plugin API and need maintainer acceptance plus baseline alignment.
  • Provider/session surfaces: 1 provider ownership API added, 1 Copilot binding schema changed. Provider routing and persisted session compatibility can affect existing Copilot runtime users during upgrade.

Stored data model
Persistent data-model change detected: serialized state: extensions/copilot/src/byok-proxy.test.ts, vector/embedding metadata: src/plugin-sdk/agent-harness-runtime.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Resolve or rerun the failed checks-node-compact-small-whole-2 shard before merge.
  • Have a maintainer explicitly accept the public SDK and provider/session compatibility surface if this is intended.

Risk before merge

  • [P1] Merging expands a public plugin SDK subpath and changes Copilot provider/session routing, so maintainers should intentionally accept that compatibility surface before landing.
  • [P1] The Copilot session binding now distinguishes BYOK auth and compact keys; existing sessions should fall back safely, but session reuse and compaction behavior remain upgrade-sensitive.
  • [P1] The current PR check rollup has one failed CI shard, so current-head CI still needs a clean rerun or maintainer disposition before merge.

Maintainer options:

  1. Accept Scoped Compatibility Risk (recommended)
    Maintainers can land as-is if they intentionally accept the expanded SDK export, BYOK provider routing, and v2 session binding behavior after resolving the failed CI shard.
  2. Keep Transport Helper Internal
    If the SDK export is not meant to become public plugin API, keep the transport metadata helpers internal and adapt Copilot through a narrower owner-local path before merge.
  3. Pause Provider Direction
    If Copilot BYOK should not be product-supported yet, pause the PR and split any reusable provider-ownership plumbing into a narrower maintainer-owned change.

Next step before merge

  • [P2] Human maintainer review should decide or accept the SDK/provider-session compatibility risk; no concrete patch defect was found for an automated repair job.

Security
Cleared: No concrete security or supply-chain defect was found; the new network path uses a loopback proxy, endpoint validation, and guarded HTTPS fetch, with no dependency or workflow changes.

Review details

Best possible solution:

Land only after owner review accepts the plugin SDK/provider-session compatibility surface and current-head validation is green or explicitly waived.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this is a PR for a new Copilot BYOK capability rather than a bug report with a current-main reproduction path.

Is this the best way to solve the issue?

Yes with maintainer acceptance: the PR uses the Copilot SDK provider contract plus OpenClaw guarded fetch and provider ownership checks instead of duplicating provider runtime policy in core.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f65aca64fc37.

Label changes

Label changes:

  • add P2: The PR is a normal bounded provider/runtime feature with tests and docs, not an emergency regression.
  • add merge-risk: 🚨 compatibility: The diff expands a public plugin SDK subpath and changes Copilot session binding compatibility behavior.
  • add merge-risk: 🚨 session-state: The diff changes SDK session reuse, compact-key matching, and persisted Copilot session binding shape.
  • add merge-risk: 🚨 auth-provider: The diff changes Copilot provider routing, BYOK credential mapping, and custom provider ownership selection.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this maintainer/member PR; the body still cites live Copilot smoke and Testbox-through-Crabbox proof.

Label justifications:

  • P2: The PR is a normal bounded provider/runtime feature with tests and docs, not an emergency regression.
  • merge-risk: 🚨 compatibility: The diff expands a public plugin SDK subpath and changes Copilot session binding compatibility behavior.
  • merge-risk: 🚨 auth-provider: The diff changes Copilot provider routing, BYOK credential mapping, and custom provider ownership selection.
  • merge-risk: 🚨 session-state: The diff changes SDK session reuse, compact-key matching, and persisted Copilot session binding shape.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply to this maintainer/member PR; the body still cites live Copilot smoke and Testbox-through-Crabbox proof.
Evidence reviewed

PR surface:

Source +1162, Tests +1434, Docs +59, Generated 0. Total +2655 across 26 files.

View PR surface stats
Area Files Added Removed Net
Source 14 1273 111 +1162
Tests 9 1445 11 +1434
Docs 2 75 16 +59
Config 0 0 0 0
Generated 1 2 2 0
Other 0 0 0 0
Total 26 2795 140 +2655

Acceptance criteria:

  • [P1] Review current-head CI after rerun, especially checks-node-compact-small-whole-2.
  • [P1] Confirm plugin SDK API baseline/export checks remain aligned.
  • [P1] Confirm focused Copilot BYOK/provider proxy tests and check:changed are green on the final head.

What I checked:

  • Live PR metadata: this PR is open, mergeable but currently unstable, labeled maintainer and size: XL, and changes 26 files with +2795/-140 LOC. (1b94b50c2747)
  • BYOK provider mapping: The branch maps non-github-copilot models into a Copilot SDK ProviderConfig, rejects unsupported endpoints and transport overrides, and fingerprints endpoint/auth/token-limit state for session compatibility. (extensions/copilot/src/provider-bridge.ts:77, 1b94b50c2747)
  • Guarded loopback proxy: The new proxy binds to 127.0.0.1, rewrites SDK-local requests to the configured provider endpoint, and sends outbound traffic through fetchWithSsrFGuard with HTTPS required. (extensions/copilot/src/byok-proxy.ts:19, 1b94b50c2747)
  • Harness ownership and session binding: The Copilot harness now accepts schemaVersion 2 bindings with byok auth mode and only supports custom BYOK providers when provider ownership is unowned and the model shape is supported. (extensions/copilot/harness.ts:93, 1b94b50c2747)
  • Harness support context: Core harness selection now passes provider ownership and resolved model provider shape into plugin support checks before forced or auto plugin harness selection. (src/agents/harness/selection.ts:158, 1b94b50c2747)
  • Public SDK seam expansion: The PR exports attachModelProviderRequestTransport and getModelProviderRequestTransport through the existing agent-harness-runtime public subpath and refreshes the committed API baseline hash. (src/plugin-sdk/agent-harness-runtime.ts:179, 1b94b50c2747)

Likely related people:

  • vincentkoc: Current-main blame and local history show recent Copilot harness and provider-surface work by this author, and the PR head commit is also theirs. (role: recent area contributor; confidence: high; commits: 1b94b50c2747, 4d034639adfd, 0fd9aa8e001e; files: extensions/copilot/src/attempt.ts, extensions/copilot/harness.ts, src/plugins/providers.ts)
  • steipete: History on provider registry, plugin ownership, and harness selection includes multiple adjacent architecture/refactor commits by Peter Steinberger. (role: adjacent owner; confidence: medium; commits: 34abb441f69c, 2d80bbc43de0, 44ec4d05de4a; files: src/plugins/providers.ts, src/agents/harness/selection.ts)
  • Pavan Kumar Gondhi: Provider ownership and trust-policy history includes a related provider hijack prevention change in the provider/plugin ownership area. (role: adjacent contributor; confidence: low; commits: 2d97eae53e21; files: src/plugins/providers.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jun 24, 2026
@vincentkoc vincentkoc force-pushed the harness-parity-audit branch from 1b94b50 to 76595ab Compare June 24, 2026 10:20
@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jun 24, 2026
@vincentkoc vincentkoc merged commit bfffc77 into main Jun 24, 2026
105 of 107 checks passed
@vincentkoc vincentkoc deleted the harness-parity-audit branch June 24, 2026 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation extensions: copilot maintainer Maintainer-authored PR merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant