Skip to content

feat(llm): import and reuse local codex model config - #56

Open
lanicc wants to merge 1 commit into
OpenBMB:mainfrom
lanicc:feat/codex-model-import
Open

feat(llm): import and reuse local codex model config#56
lanicc wants to merge 1 commit into
OpenBMB:mainfrom
lanicc:feat/codex-model-import

Conversation

@lanicc

@lanicc lanicc commented Jul 31, 2026

Copy link
Copy Markdown

No description provided.

@hm1229
hm1229 self-requested a review August 1, 2026 09:24
@hm1229 hm1229 self-assigned this Aug 1, 2026
@hm1229

hm1229 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the contribution. The Responses API implementation is generally heading in the right direction. I also rechecked the message mapping against the official Responses API documentation: using input_text for user/developer messages and output_text for assistant history is correct.
However, I think the following issues should be addressed before merging:
[P1] Do not expose the backend process’s Codex credentials through a tenant-scoped API
The import endpoint is available to any tenant administrator, but load_local_codex_model_config() reads $CODEX_HOME/config.toml from the backend process’s filesystem.
In a desktop-only deployment, that may happen to be the current user’s config. In a server or multi-tenant deployment, however, it is the service account/server operator’s Codex config. A tenant administrator could import that credential into their tenant and use the associated provider quota.
Please restrict this functionality to an explicitly verified local/desktop mode, or move the import to a trusted client-side/desktop flow. A normal tenant API should never read credentials from the server process’s home directory.
Relevant code: model_configs.py, codex_config.py
[P2] A display name is not a safe upsert key
The import searches for an existing model using only tenant_id + name, then overwrites the most recently updated matching row.
Model names are user-editable and are not unique source identifiers. An unrelated manually created configuration named Codex could therefore have its endpoint, model, credential, verification state, enabled state, and default state overwritten simply by clicking “Import Codex”.
Please store a stable import/source identifier, such as the Codex provider ID, or create a new configuration and require explicit confirmation before replacing an existing one.
Relevant code: model_configs.py
[P2] The importer supports only one Codex authentication mode
The loader requires experimental_bearer_token to be present directly in config.toml. Valid Codex custom providers may instead use env_key, requires_openai_auth, or command-backed authentication. Those configurations will currently fail with CODEX_CONFIG_API_KEY_UNAVAILABLE, even though the UI presents this as a general “Import Codex” feature.
Please either support a clearly defined safe subset of Codex authentication methods or rename/document the feature as importing only custom providers with an inline bearer token. OpenAI authentication or keychain credentials should not be copied into StaffDeck implicitly.
Relevant code: codex_config.py, Codex custom provider documentation
Finally, this head is currently 15 commits behind main. It merges cleanly in a merge-tree check, but the branch should still be updated and the full test suite rerun against the latest Harness v2 changes before merging.

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.

2 participants