Skip to content

Fix Codex ACP model selection#879

Merged
binaricat merged 1 commit into
mainfrom
codex/fix-875-codex-model-picker
May 1, 2026
Merged

Fix Codex ACP model selection#879
binaricat merged 1 commit into
mainfrom
codex/fix-875-codex-model-picker

Conversation

@binaricat
Copy link
Copy Markdown
Owner

Summary

  • load Codex ACP model options dynamically instead of relying on the static preset list
  • support ACP config option model/reasoning selectors when building the picker
  • keep custom Codex configs from falling back to stale hardcoded models

Fixes #875

Tests

  • node --test electron/bridges/ai/acpModels.test.cjs
  • npm run lint
  • npm test
  • npm run build

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef5effd037

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread electron/bridges/ai/acpModels.cjs Outdated
Comment on lines +35 to +37
const modelValues = Array.isArray(modelOption?.options)
? modelOption.options.map(normalizeConfigOptionValue).filter(Boolean)
: [];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Flatten grouped ACP model options before parsing

normalizeConfigOptionsModels assumes modelOption.options is a flat array of values, but ACP allows SessionConfigSelectOptions to be grouped (SessionConfigSelectGroup with nested options). When an agent returns grouped model options, normalizeConfigOptionValue receives group objects (no value), all entries are filtered out, and modelValues becomes empty, so we silently fall back to legacy availableModels and lose the runtime model catalog this change is meant to use.

Useful? React with 👍 / 👎.

@binaricat binaricat force-pushed the codex/fix-875-codex-model-picker branch 2 times, most recently from 9b03a5c to 82914dc Compare April 30, 2026 16:18
@binaricat binaricat force-pushed the codex/fix-875-codex-model-picker branch from 82914dc to 784343a Compare April 30, 2026 16:26
@binaricat binaricat merged commit 20694a4 into main May 1, 2026
17 checks passed
@binaricat binaricat deleted the codex/fix-875-codex-model-picker branch May 10, 2026 15:33
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.

Bug: Codex Cli无法指定模型

1 participant