You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.