Skip to content

Feature/provider custom headers pr#1

Open
alindsilva wants to merge 23 commits intomainfrom
feature/provider-custom-headers-pr
Open

Feature/provider custom headers pr#1
alindsilva wants to merge 23 commits intomainfrom
feature/provider-custom-headers-pr

Conversation

@alindsilva
Copy link
Copy Markdown
Owner

Summary

Merge the custom headers feature from the upstream PR docker#2108 branch into this fork's main branch for personal use while waiting for upstream merge.

What's Included

Features

  • ✅ Custom headers support for ProviderConfig and ModelConfig
  • ✅ Environment variable expansion in headers (${VAR_NAME})
  • ✅ Cloudflare AI Gateway support
  • ✅ Headers merging (model-level overrides provider-level)

Bug Fixes

  • ✅ Fixed function scope bug in applyProviderDefaults (commit 6d6d986)
  • ✅ Fixed 11 lint errors (gci, gocritic, modernize, testifylint, gofmt)
  • ✅ Resolved merge conflicts from 339 commits sync
  • ✅ Updated devcontainer to use mise instead of go-task

Documentation

  • ✅ Comprehensive troubleshooting guide in commit 5e55a09
  • ✅ Example configuration in examples/custom_provider.yaml

Testing

All tests passing:

go test ./pkg/model/provider/...

Build successful:

mise build

Configuration Example

# No schema_version - uses latest automatically

providers:
  google-ai-studio:
    api_type: openai_chatcompletions
    base_url: https://gateway.ai.cloudflare.com/v1/${ACCOUNT}/${GATEWAY}/compat
    headers:
      cf-aig-authorization: Bearer ${CLOUDFLARE_AI_GATEWAY_TOKEN}
      x-goog-api-key: ${GOOGLE_API_KEY}

models:
  gemini_flash:
    provider: google-ai-studio
    model: google-ai-studio/gemini-3-flash-preview

Related

Commits

  • cce9e24: fix: address lint errors from CI/CD pipeline
  • 60aa6ae: Merge main into feature/provider-custom-headers-pr
  • 2b0d2fa: fix: resolve merge conflict - add missing alias handling section
  • d688dde: chore: update devcontainer to use mise instead of go-task
  • 6d6d986: fix: correct function scope in applyProviderDefaults
  • 5e55a09: docs: merge troubleshooting guide for custom headers PR

Add Headers map[string]string to ProviderConfig, allowing custom HTTP
headers on provider definitions. Headers flow through ProviderOpts to
the OpenAI client with env var expansion (${VAR_NAME} syntax).

Includes:
- ProviderConfig.Headers field in config schema (v3 and latest)
- Headers wiring in applyProviderDefaults
- OpenAI client: headers parsing, env expansion, auth middleware
  for custom providers without token_key
- Schema normalization (normalizeUnionTypes) for gateway compatibility
- Handle both map[string]string and map[interface{}]interface{} YAML types
The filter, instructions, and toon toolset wrappers were not forwarding
Start() and Stop() calls to their inner toolsets. This caused MCP tools
to fail with 'toolset not started' errors in multi-agent configurations.
- Convert anyOf patterns like {anyOf: [{type:string},{type:null}]} to
  {type:string} for compatibility with AI gateways (e.g. Cloudflare)
  that don't support anyOf in tool parameter schemas.
- Log HTTP response body on non-2xx API errors for easier debugging.
Add custom headers support and ${VAR_NAME} expansion in base_url to the
Gemini and Anthropic provider clients, matching the existing OpenAI
client capability. Also add Headers field directly to ModelConfig for
convenience (no separate providers section needed).

- Gemini: read headers from ProviderOpts, expand env vars, set on
  genai.HTTPOptions; expand env vars in base_url
- Anthropic: same pattern with option.WithHeader; expand env vars
  in base_url
- ModelConfig.Headers: new field merged into ProviderOpts['headers']
  with model-level taking precedence over provider-level
- Updated JSON schema and config types (v3 + latest)
…_url env expansion for providers

* feature/provider-custom-headers-pr:
  feat: add custom headers and base_url env expansion to all providers
  fix: normalize anyOf schemas and add API error response body logging
  fix: forward Start/Stop to inner toolsets in teamloader wrappers
  feat: add custom headers support for provider configs
… feedback

- Add Headers field to ModelConfig and ProviderConfig in v4, v5, v6 config
  types so headers survive the JSON upgrade chain from v3 to latest. This
  was causing Cloudflare AI Gateway 401 errors because custom headers were
  silently dropped at the v3→v4 boundary.

- Address Copilot review comments on PR docker#2108:
  1. Remove response body from streaming error logs to avoid leaking
     sensitive data (pkg/runtime/streaming.go)
  2. Deep-copy provider headers map before merging to avoid mutating
     shared config across models (pkg/model/provider/provider.go)
  3. Gather env vars from model-level base_url in addition to provider
     base_url (pkg/config/gather.go)
  4. Expand env vars in OpenAI/Azure base_url consistently with
     Anthropic/Gemini (pkg/model/provider/openai/client.go)
  5. Redact header values from error logs to prevent credential leaks
     (pkg/model/provider/openai/client.go)
  6. Tighten union type normalization to only collapse nullable patterns
     (exactly 2 options with one being null), preserving non-nullable
     unions (pkg/model/provider/openai/schema.go)
- Fix import ordering (gci) in custom_headers_test.go and openai/client.go
- Replace if-else-if chains with switch statements (gocritic)
- Use maps.Copy instead of manual loops (modernize)
- Fix nil return in normalizeUnionTypes (gocritic)
- Replace else-if with else if in custom_headers_test.go (gocritic)
- Use assert.Empty instead of assert.Equal with empty string (testifylint)
- Remove extra blank line (gofmt)
Sync PR branch with 339 commits from upstream main.
Resolved conflict in provider.go by keeping headers logic and adapting to
new function signature (changed from pointer to value return).
After merge with main, the built-in alias handling section was incomplete.
Fixed by properly structuring the function:
- Custom provider handling (with headers) → early return
- Built-in alias handling (Aliases map lookup)
- General header merging for non-custom providers
- Model defaults and return
The project migrated from Taskfile to mise in commit 2bf2683 (March 2026),
but the devcontainer.json was not updated. This commit:

- Replaces go-task feature with mise-en-dev feature
- Adds custom Dockerfile to fix yarn repository issue
- Enables 'mise cross' and other mise commands in devcontainer

This aligns the development environment with the current build system.
The merge conflict resolution introduced an extra closing brace that
terminated applyProviderDefaults() prematurely. This caused all code
after the custom provider block (alias handling and header merging for
non-custom providers) to become orphaned code outside the function.

This broke configurations using built-in providers (like 'google') with
custom base_url and headers (e.g., Cloudflare AI Gateway), since the
header merging code was unreachable.

Fixed by removing the erroneous closing brace and ensuring proper
nesting of the conditional blocks.
TROUBLESHOOTING NOTES FOR FUTURE MERGES WITH UPSTREAM
======================================================

## Critical Merge Pitfall: Function Scope Bug (April 4, 2026)

**Issue**: Merge conflict in pkg/model/provider/provider.go introduced an extra
closing brace that terminated applyProviderDefaults() prematurely, making all
code after line 351 unreachable (alias handling, header merging for non-custom
providers).

**Symptoms**:
- Headers don't work even though code looks correct
- 401 Unauthorized errors from Cloudflare AI Gateway
- Custom provider headers work, built-in provider headers fail

**Root Cause**:
Incorrect closing brace placement after custom provider block:
  applyModelDefaults(enhancedCfg)
  return enhancedCfg
  }
}  # ← WRONG: This extra } closes the entire function!

if alias, exists := Aliases[cfg.Provider]; exists {
  // This code becomes unreachable!

**Fix**: Remove the extra closing brace - the function should have only ONE
final closing brace at the end.

**Detection**:
- Run: sed -n '290,376p' pkg/model/provider/provider.go | cat -A
- Check indentation: Headers code needs 2-3 tabs, not at column 0
- Test: Custom provider should work, model-level headers should also work

## Custom Headers Feature Architecture

**Config Schema Versions**:
- v7 and earlier: NO Headers field in ModelConfig or ProviderConfig
- latest (unreleased): HAS Headers in both ModelConfig and ProviderConfig
- User configs should OMIT schema_version to use 'latest' automatically
- Setting schema_version: "7" will break headers with 'unknown field' error

**Code Flow**:
1. Config parsing → applyProviderDefaults() in pkg/model/provider/provider.go
2. Custom providers (from providers: section) → headers copied to ProviderOpts
3. Model-level headers merged second (takes precedence over provider headers)
4. Headers stored in enhancedCfg.ProviderOpts["headers"] as map[string]string
5. Each client (openai, anthropic, gemini) reads from ProviderOpts["headers"]
6. Clients expand environment variables () before sending requests

**Key Functions**:
- applyProviderDefaults(): Merges custom provider defaults and model headers
- cloneModelConfig(): Deep copies ModelConfig including ProviderOpts map
- OpenAI/Anthropic/Gemini clients: Read headers from cfg.ProviderOpts["headers"]

## Cloudflare AI Gateway Configuration

**Correct Setup** (using custom provider):
providers:
  google-ai-studio:
    api_type: openai_chatcompletions  # Required for /compat endpoint
    base_url: https://gateway.ai.cloudflare.com/v1/${ACCOUNT}/${GATEWAY}/compat
    headers:
      cf-aig-authorization: Bearer ${CLOUDFLARE_AI_GATEWAY_TOKEN}
      x-goog-api-key: ${GOOGLE_API_KEY}

models:
  gemini_flash:
    provider: google-ai-studio  # References custom provider
    model: google-ai-studio/gemini-3-flash-preview

**Why This Works**:
- Cloudflare /compat endpoint expects OpenAI-compatible API format
- provider: google would use Gemini SDK (native format) - won't work!
- Custom provider with api_type: openai_chatcompletions routes to OpenAI client
- OpenAI client applies headers from ProviderOpts before making requests

**Common Mistakes**:
- Using provider: google instead of custom provider → Gemini SDK used
- Missing api_type: openai_chatcompletions → wrong API format
- Setting schema_version: "7" → headers field rejected
- Undefined environment variables → 401 Unauthorized (empty header values)

## Testing After Merge

1. Verify function structure:
   grep -A60 'func applyProviderDefaults' pkg/model/provider/provider.go

2. Count closing braces (should match):
   sed -n '290,380p' pkg/model/provider/provider.go | grep -c '^[[:space:]]*}$'

3. Test header merging (create temporary test):
   - Custom provider headers should be in ProviderOpts
   - Model-level headers should merge and override provider headers
   - Environment variables should remain as ${VAR} (expanded by clients)

4. Build and run integration test:
   mise build
   ./bin/docker-agent run examples/custom_provider.yaml --debug

5. Check debug logs for:
   - "Applying custom headers"
   - "Applied custom header"
   - Header count should match config

## Build System Notes

- Project migrated from Taskfile to mise on March 28, 2026 (commit 2bf2683)
- DevContainer updated from go-task to mise-en-dev feature
- Use 'mise build' not 'task build'
- Go 1.26.1+ required (use GOTOOLCHAIN=auto if local version too old)

## Upstream Status (as of April 4, 2026)

- docker/docker-agent:main does NOT have Headers field yet
- PR docker#2108 (feature/provider-custom-headers-pr) adds this capability
- Until merge, only this fork supports custom headers
- After merge, remove this commit or keep for historical reference

## Related Commits

- cce9e24: Initial lint fixes for PR
- 60aa6ae: Merged 339 commits from main (March 28 → April 4, 2026)
- 2b0d2fa: First merge conflict resolution (introduced the bug)
- 6d6d986: Fixed function scope bug (removed extra closing brace)
- 8d15048: Added Headers to v4/v5/v6 config types (config upgrade support)

## References

- PR: docker#2108
- Schema: ./agent-schema.json
- Config types: ./pkg/config/latest/types.go
- Provider logic: ./pkg/model/provider/provider.go
- Example config: ./examples/custom_provider.yaml
Copilot AI review requested due to automatic review settings April 4, 2026 13:04
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR merges upstream support for configuring custom HTTP headers on providers/models (with ${VAR} expansion), adds Cloudflare AI Gateway-related compatibility tweaks, and fixes toolset lifecycle propagation through several teamloader wrappers.

Changes:

  • Add headers support to config schema/types (provider + model) and gather referenced env vars from headers/base URLs.
  • Apply/merge provider-level + model-level headers into provider options and propagate headers/base URL expansion into provider clients (OpenAI/Anthropic/Gemini).
  • Ensure Start/Stop lifecycle calls propagate through teamloader toolset wrappers (filter/instructions/toon) and add/adjust tests.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
pkg/teamloader/toon.go Forward Start/Stop to wrapped toolset when supported.
pkg/teamloader/toon_test.go Tests for Start/Stop forwarding behavior in toon wrapper.
pkg/teamloader/instructions.go Forward Start/Stop through instruction wrapper.
pkg/teamloader/instructions_test.go Tests for instruction wrapper Start/Stop forwarding.
pkg/teamloader/filter.go Forward Start/Stop through tool filtering wrapper (important for MCP init).
pkg/teamloader/filter_test.go Add startable mock + forwarding/integration tests.
pkg/runtime/streaming.go Add debug logging for OpenAI API error status code on stream recv errors.
pkg/model/provider/schema_test.go Update expected schema output related to union/nullable types.
pkg/model/provider/provider.go Copy/merge provider/model headers into ProviderOpts during default application.
pkg/model/provider/openai/schema.go Add schema normalization to remove union/nullable constructs for gateway compatibility.
pkg/model/provider/openai/schema_test.go Add tests for normalizeUnionTypes anyOf normalization behavior.
pkg/model/provider/openai/client.go Expand base_url, apply custom headers, and change no-token_key auth behavior.
pkg/model/provider/openai/api_type_test.go Update expectation: Authorization header stripped when no token_key.
pkg/model/provider/gemini/client.go Expand base_url and pass custom headers via genai.HTTPOptions.
pkg/model/provider/custom_headers_test.go New tests for header propagation/merging in applyProviderDefaults.
pkg/model/provider/anthropic/client.go Expand base_url and apply custom headers via request options.
pkg/config/v6/types.go Add Headers fields to provider/model config types (v6).
pkg/config/v5/types.go Add Headers fields to provider/model config types (v5).
pkg/config/v4/types.go Add Headers fields to provider/model config types (v4).
pkg/config/v3/types.go Add Headers fields to provider/model config types (v3).
pkg/config/latest/types.go Add Headers fields to provider/model config types (latest).
pkg/config/gather.go Gather env vars referenced in headers and base URLs ($VAR / ${VAR}).
examples/custom_provider.yaml Update example to show Cloudflare AI Gateway headers usage.
agent-schema.json Add headers to ProviderConfig/ModelConfig schema.
.devcontainer/Dockerfile New devcontainer Dockerfile; removes invalid Yarn apt repo.
.devcontainer/devcontainer.json Switch devcontainer to build Dockerfile and use mise feature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +227 to +240
var nonNullType string
for _, item := range anyOf {
if itemMap, ok := item.(map[string]any); ok {
if typStr, ok := itemMap["type"].(string); ok {
if typStr == "null" {
hasNull = true
} else {
nonNullType = typStr
}
}
}
}
if hasNull && nonNullType != "" {
schema["type"] = nonNullType
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The anyOf normalization drops all schema keywords from the non-null branch. For patterns like {"anyOf": [{"type":"object","properties":...},{"type":"null"}]}, the current logic sets type="object" and deletes anyOf but does not copy properties/required/etc from the object variant, producing an incomplete schema. Consider promoting the full non-null variant map into the current node (merge/copy its keys) before removing anyOf so object/array constraints are preserved.

Suggested change
var nonNullType string
for _, item := range anyOf {
if itemMap, ok := item.(map[string]any); ok {
if typStr, ok := itemMap["type"].(string); ok {
if typStr == "null" {
hasNull = true
} else {
nonNullType = typStr
}
}
}
}
if hasNull && nonNullType != "" {
schema["type"] = nonNullType
var nonNullSchema map[string]any
for _, item := range anyOf {
if itemMap, ok := item.(map[string]any); ok {
if typStr, ok := itemMap["type"].(string); ok {
if typStr == "null" {
hasNull = true
} else {
nonNullSchema = itemMap
}
}
}
}
if hasNull && nonNullSchema != nil {
maps.Copy(schema, nonNullSchema)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot I have fixed the code myself for this reported issue [https://github.com//pull/1#discussion_r3035568005] please verify and resolve conversation if you agree

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix looks correct. schema.go now uses var nonNullSchema map[string]any to capture the full non-null branch and calls maps.Copy(schema, nonNullSchema) before deleting anyOf, which correctly promotes all keys (properties, required, items, etc.) — not just type. Conversation can be resolved.

Comment on lines +73 to +85
// Custom provider without token_key - prevent SDK from using OPENAI_API_KEY env var
// We need to explicitly set the API key to prevent the SDK from reading OPENAI_API_KEY
// but we don't want to send an Authorization header. The SDK doesn't send the header
// if we use option.WithAPIKey with a specific marker value and then remove it via middleware.
slog.Debug("Custom provider with no token_key, disabling OpenAI SDK authentication",
"provider", cfg.Provider, "base_url", cfg.BaseURL)
clientOptions = append(clientOptions, option.WithAPIKey(""))

// Use a custom HTTP client that removes the Authorization header
clientOptions = append(clientOptions, option.WithMiddleware(func(req *http.Request, next option.MiddlewareNext) (*http.Response, error) {
// Remove Authorization header for custom providers without token_key
req.Header.Del("Authorization")
return next(req)
}))
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the custom-provider-without-token_key path, the comment says we "explicitly set the API key to prevent the SDK from reading OPENAI_API_KEY", but the code only deletes the Authorization header in middleware. This still allows the WebSocket path (buildWSHeaderFn) to fall back to OPENAI_API_KEY and send Authorization during the handshake, which defeats the intent of disabling auth for custom providers. Align WebSocket auth behavior with the HTTP path (e.g., avoid OPENAI_API_KEY fallback for custom providers without token_key, or strip/override in the WS header function).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Comment on lines 89 to +114
@@ -89,7 +106,63 @@ func NewClient(ctx context.Context, cfg *latest.ModelConfig, env environment.Pro
}
}
} else if cfg.BaseURL != "" {
clientOptions = append(clientOptions, option.WithBaseURL(cfg.BaseURL))
expandedBaseURL, err := environment.Expand(ctx, cfg.BaseURL, env)
if err != nil {
return nil, fmt.Errorf("expanding base_url: %w", err)
}
clientOptions = append(clientOptions, option.WithBaseURL(expandedBaseURL))
}
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base_url is now expanded before being passed to option.WithBaseURL, but later WebSocket pool initialization uses cfg.BaseURL directly (unexpanded) when computing httpToWSURL(baseURL). If cfg.BaseURL contains ${...} placeholders, SSE will work but WebSocket transport will build an invalid WS URL. Consider reusing the expanded base URL (or expanding again) for the wsPool base URL computation.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 9f8e453. Moved expandedBaseURL to outer scope (before the if gateway == "" block) so it's accessible for the WebSocket pool initialization. The wsPool now uses cmp.Or(expandedBaseURL, "https://api.openai.com/v1") instead of the raw cfg.BaseURL, ensuring ${...} placeholders are resolved before computing the WebSocket URL.

Comment on lines +333 to +339
if len(enhancedCfg.Headers) > 0 {
existing, _ := enhancedCfg.ProviderOpts["headers"].(map[string]string)
if existing == nil {
existing = make(map[string]string)
}
maps.Copy(existing, enhancedCfg.Headers)
enhancedCfg.ProviderOpts["headers"] = existing
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cloneModelConfig only shallow-copies ProviderOpts, so if the incoming config already has ProviderOpts["headers"] as a map[string]string, existing will alias the caller's map. The subsequent maps.Copy(existing, enhancedCfg.Headers) mutates that shared map, violating the function doc that the caller's ModelConfig/ProviderOpts are never mutated. Consider always copying existing into a new map before merging and reassigning it back into enhancedCfg.ProviderOpts["headers"].

Suggested change
if len(enhancedCfg.Headers) > 0 {
existing, _ := enhancedCfg.ProviderOpts["headers"].(map[string]string)
if existing == nil {
existing = make(map[string]string)
}
maps.Copy(existing, enhancedCfg.Headers)
enhancedCfg.ProviderOpts["headers"] = existing
// Always merge into a fresh map so we never mutate a caller-owned headers map
// through the shallow-copied ProviderOpts.
if len(enhancedCfg.Headers) > 0 {
existing, _ := enhancedCfg.ProviderOpts["headers"].(map[string]string)
mergedHeaders := make(map[string]string, len(existing)+len(enhancedCfg.Headers))
if existing != nil {
maps.Copy(mergedHeaders, existing)
}
maps.Copy(mergedHeaders, enhancedCfg.Headers)
enhancedCfg.ProviderOpts["headers"] = mergedHeaders

Copilot uses AI. Check for mistakes.
alindsilva and others added 5 commits April 4, 2026 13:57
- Fix gci formatting in provider.go (proper indentation of headers code)
- Fix gocritic else-if in custom_headers_test.go

Note: Upstream lint errors in pkg/teamloader/* are not addressed as
they are not part of the custom headers feature.
During previous merge conflict resolution, duplicate code was left at lines
330-340 that caused a 'non-declaration statement outside function body' error.

The custom provider block had an incorrect early return that prevented the
common code (alias handling and model-level headers merge) from executing.

This fix removes the orphaned code and allows the function to flow properly:
1. Handle custom providers (copy provider-level headers)
2. Handle aliases (set base URL and token key)
3. Merge model-level headers for ALL providers
4. Apply model defaults and return

Fixes CI error: provider.go:343:2: syntax error: non-declaration statement
outside function body (typecheck)
Apply Copilot review suggestion to fix mutability issue. Previously, when
merging model-level headers into provider_opts, the code would mutate the
original caller's map if it already contained headers.

Now always creates a fresh map and copies both existing headers and
model-level headers into it, maintaining the documented side-effect-free
behavior of applyProviderDefaults.

Co-authored-by: GitHub Copilot <[email protected]>
…om/alindsilva/docker-agent into feature/provider-custom-headers-pr

* 'feature/provider-custom-headers-pr' of https://github.com/alindsilva/docker-agent:
  Apply suggestion from @Copilot
Copy link
Copy Markdown
Owner Author

@alindsilva alindsilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. Couple of suggestions for copilot accepted.

Apply Copilot review suggestion to fix incomplete schema handling. Previously,
when normalizing anyOf patterns like:
  {"anyOf": [{"type":"object","properties":{...}},{"type":"null"}]}

The code would only copy the 'type' field, losing properties, required,
items, and other schema constraints from the non-null variant.

Now copies the complete non-null schema map to preserve all constraints,
ensuring Gemini/Cloudflare compatibility without losing type information.

Co-authored-by: GitHub Copilot <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +77 to +81
// Custom provider without token_key - prevent SDK from using OPENAI_API_KEY env var
// We need to explicitly set the API key to prevent the SDK from reading OPENAI_API_KEY
// but we don't want to send an Authorization header. The SDK doesn't send the header
// if we use option.WithAPIKey with a specific marker value and then remove it via middleware.
slog.Debug("Custom provider with no token_key, disabling OpenAI SDK authentication",
Copy link

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For custom providers without token_key, the HTTP middleware removes the Authorization header, but the WebSocket handshake path still builds headers via buildWSHeaderFn() which falls back to OPENAI_API_KEY when TokenKey is empty. If provider_opts.transport=websocket is used, this can leak the user's OPENAI_API_KEY to the custom provider during the WS handshake. Update the WS header logic to mirror the HTTP behavior for custom providers without token_key (i.e., do not fall back to OPENAI_API_KEY, or explicitly return an empty Authorization header in this case). Also, the comment here about “explicitly set the API key to prevent the SDK from reading OPENAI_API_KEY” is no longer accurate since no API key is set.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 5e79266. Updated buildWSHeaderFn to mirror the HTTP client auth logic: custom providers without token_key no longer fall back to OPENAI_API_KEY (the else if !isCustomProvider(&c.ModelConfig) guard ensures only non-custom providers get the OPENAI_API_KEY fallback). Also fixed the stale comment in NewClient that inaccurately described setting a dummy API key.

Aligns devcontainer naming with the current project name.
…om/alindsilva/docker-agent into feature/provider-custom-headers-pr

* 'feature/provider-custom-headers-pr' of https://github.com/alindsilva/docker-agent:
  fix: align WebSocket auth with HTTP for custom providers without token_key
  Update examples/custom_provider.yaml
  fix: use expanded base URL for WebSocket pool initialization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants