Skip to content

fix: support --model for codex sessions#192

Merged
osolmaz merged 4 commits intomainfrom
codex/land-pr-128
Mar 29, 2026
Merged

fix: support --model for codex sessions#192
osolmaz merged 4 commits intomainfrom
codex/land-pr-128

Conversation

@osolmaz
Copy link
Copy Markdown
Contributor

@osolmaz osolmaz commented Mar 29, 2026

Summary

  • preserve the core fix from #128: apply Codex --model after session/new via session/set_config_option
  • keep the requested model value unchanged instead of rewriting aliases
  • include the small landing fixes needed on top of current main

Context

This PR supersedes #128 only because the original fork branch could not be updated from this maintainer environment even though the PR was marked editable.

Original author and core fix: @Asm3r96
Original PR: #128

Validation

  • pnpm run check:docs
  • focused CLI regression: prompt reconciles agentSessionId from loadSession metadata
  • local pnpm run test:coverage and pnpm run check still hit the same repo-local coverage baseline failure seen on main in this environment under the only installed local Node runtime, so they are not useful as change-specific signal here

@osolmaz osolmaz merged commit 6fc4d65 into main Mar 29, 2026
1 check passed
@osolmaz osolmaz deleted the codex/land-pr-128 branch March 29, 2026 08:59
@osolmaz
Copy link
Copy Markdown
Contributor Author

osolmaz commented Mar 29, 2026

Landed from this replacement branch because the original fork-backed PR #128 could not be updated from the maintainer environment.\n\n- Original author: @Asm3r96\n- Merge commit: 6fc4d65\n\nThanks @Asm3r96!

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: f70e5e7c6a

ℹ️ 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 on lines +12 to +15
if (commandToken === "codex-acp") {
return true;
}
return args.some((arg) => arg.includes("codex-acp"));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recognize codex-acp script paths as Codex sessions

isCodexAcpCommand only treats a command as Codex when the basename is exactly codex-acp (with only .cmd/.exe/.bat stripped) or when an argument contains codex-acp. If agents.codex.command is configured to a direct script path like /opt/tools/codex-acp.js with no extra args, this check returns false, so AcpClient.createSession never sends session/set_config_option for model and --model is silently ignored for that Codex setup.

Useful? React with 👍 / 👎.

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