Skip to content

Add support for optional api_key configuration for declarative openai-engine providers#9202

Open
monroewilliams wants to merge 3 commits into
aaif-goose:mainfrom
monroewilliams:declarative-optional-api-key
Open

Add support for optional api_key configuration for declarative openai-engine providers#9202
monroewilliams wants to merge 3 commits into
aaif-goose:mainfrom
monroewilliams:declarative-optional-api-key

Conversation

@monroewilliams
Copy link
Copy Markdown
Contributor

@monroewilliams monroewilliams commented May 14, 2026

Summary

Adds a nonempty api_key_env configuration to the lmstudio, llama_swap, and omlx declarative providers.

These providers all set requires_auth=false already (since auth is optional for local inference), but logic elsewhere in the code coupled api_key_env to requires_auth, causing issues for providers where api_key_env is non-empty and requires_auth=false:

  • the configuration UI would not allow the user to set the api key
  • if the key was configured through the config file it would still not be sent to the endpoint

The PR fixes both of these issues by adjusting the logic to decouple api_key_env from requires_auth and handle this case correctly.

Changes:

  • lmstudio.json: Set api_key_env to LMSTUDIO_API_KEY

  • llama_swap.json: Set api_key_env to LLAMA_SWAP_API_KEY

  • omlx.json: Set api_key_env to OMLX_API_KEY

  • declarative_providers.rs: Update llama_swap api_key_env test assertion

  • provider_registry.rs: Expose the api_key_env config in UI regardless of requires_auth setting

  • openai.rs: Read the API key when configured, regardless of requires_auth. For the requires_auth=false case:

    • handle NotFound as not an error (the user didn't provide the optional api key)
    • warn on keyring errors instead of failing (the api key isn't needed, so this isn't fatal)
  • inventory/mod.rs: Include api_key_env in identity hash so model cache invalidates when key changes, regardless of the value of requires_auth.

Testing

  • Manual test: configured the oMLX provider to talk to a locally running oMLX instance that has a required API key set, and verified that it's able to fetch the model list and run prompts.
  • Added unit test cases in openai.rs that cover various combinations of required/not required and key presence/absence/store errors.
  • Making this logic deterministically unit testable required a minor refactor to extract the logic for key resolution into a distinct method with an interface that allows the tests to mock the key provider.

Related Issues

N/A

Screenshots/Demos (for UX changes)

Before:
Screenshot 2026-05-13 at 9 14 39 PM
Screenshot 2026-05-13 at 9 21 41 PM

After:
Screenshot 2026-05-13 at 9 27 21 PM
Screenshot 2026-05-13 at 9 27 44 PM

…mlx declarative providers.

These providers all set requires_auth=false since auth is optional for local inference, but due to logic elsewhere in the code, openai-engine based providers' api keys could not be configured through the UI and would not be sent to the endpoint unless requires_auth was set to true.

To fix this, also correct the logic in several places so that declarative providers based on the openai engine which define a nonempty api_key_env and have requires_auth=false allow the key to be set through the UI and will send the key to the endpoint.

Changes:
- lmstudio.json: Set api_key_env to LMSTUDIO_API_KEY
- llama_swap.json: Set api_key_env to LLAMA_SWAP_API_KEY
- omlx.json: Set api_key_env to OMLX_API_KEY
- declarative_providers.rs: Update llama_swap api_key_env test assertion

- provider_registry.rs: Expose api_key_env config in UI regardless of requires_auth setting
- openai.rs: Read API key when configured, regardless of requires_auth. For the requires_auth=false case:
-- handle NotFound as not an error (the user didn't provide the optional api key)
-- warn on keyring errors instead of failing (the api key isn't needed, so this isn't fatal)
- inventory/mod.rs: Include api_key_env in identity hash so model cache invalidates when key changes, regardless of the value of requires_auth.
…t tests

The api_key resolution logic in from_custom_config was hard to test
deterministically because it called Config::global() directly. Extract it into a pure
static method resolve_api_key that takes a closure for secret lookup,
making it testable with mocked secrets. Add 6 unit tests covering
all branches: empty env, missing key (required vs optional), present
key, and other errors (required vs optional).

Co-Authored-By: Qwen3.6-35B-A3B
@monroewilliams monroewilliams force-pushed the declarative-optional-api-key branch from 19bed4f to 7db99f6 Compare May 14, 2026 06:02
@monroewilliams
Copy link
Copy Markdown
Contributor Author

monroewilliams commented May 14, 2026

@DOsinga: here's the next one from my stack.
I made pretty heavy use of Qwen3.6 on this one, since my rust experience is limited. (I'm more of a C++/Objective-C/Swift guy ;) )
You mentioned on my earlier PR that this was probably a light enough change that it didn't need an issue attached, but I can open one if you think it's warranted.

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.

1 participant