Skip to content

fix: fallback to provider credentials for usage script tests#1689

Closed
Alexlangl wants to merge 5 commits into
farion1231:mainfrom
Alexlangl:fix/provider-usage-1658
Closed

fix: fallback to provider credentials for usage script tests#1689
Alexlangl wants to merge 5 commits into
farion1231:mainfrom
Alexlangl:fix/provider-usage-1658

Conversation

@Alexlangl
Copy link
Copy Markdown
Contributor

@Alexlangl Alexlangl commented Mar 26, 2026

概要

测试用量脚本没有复用 provider 中已有的 apiKey / baseUrl,当这些字段留空时会直接使用空值执行。

复现

  1. provider 中已配置 apiKey / baseUrl
  2. 测试脚本时将这两个字段留空
  3. 执行测试

结果:不会回退到 provider 配置,而是使用空值

修复

  • 测试路径补齐凭证回退逻辑
  • apiKey / baseUrl 为空(或仅空白)时,自动使用 provider 配置
  • 保持显式传入参数优先(不影响 override)

支持来源:

  • 顶层 apiKey
  • 顶层 baseUrl / baseURL
  • options.apiKey
  • options.baseURL
  • env.*

验证

cargo test resolve_usage_credentials --manifest-path src-tauri/Cargo.toml -- --nocapture
cargo test --manifest-path src-tauri/Cargo.toml

Related

Fix #1658

Copy link
Copy Markdown
Owner

@farion1231 farion1231 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

Repository owner deleted a comment from chatgpt-codex-connector Bot Mar 26, 2026
Copy link
Copy Markdown
Owner

@farion1231 farion1231 left a comment

Choose a reason for hiding this comment

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

感谢您的贡献,请查看一下有没有以下问题:
第一,新的 fallback helper 没有覆盖 Codex 现有的标准配置结构,也就是 auth.OPENAI_API_KEYconfig TOML 里的 base_url,这会导致 Codex 在“测试用量脚本”时如果把凭证输入框留空,仍然解析成空字符串,和本 PR “回退到 provider 凭证”的目标不一致;
第二,helper 也丢掉了仓库其他路径已经兼容的 api_key / base_url 别名,因此一部分旧配置或 additive config 依然无法走到 fallback。除此之外,这个 PR 还引入了几处需要在描述里明确说明的行为变化:test_usage_script 现在从“仅使用传入参数”变成“查库后与 provider 配置合并”,凭证优先级也从原先偏向 env.* 变成顶层 apiKey / options.apiKey 优先,同时顶层和 options 里的 baseUrl 现在也会统一去掉尾部 /;这些不一定是问题,但都属于用户可感知的兼容性变化。
另有一个小的实现建议是 test_usage_script 可以直接使用现有的 get_provider_by_id,避免先加载整个 app 下的所有 provider。

@Alexlangl
Copy link
Copy Markdown
Contributor Author

Alexlangl commented Mar 28, 2026

感谢您的贡献,请查看一下有没有以下问题: 第一,新的 fallback helper 没有覆盖 Codex 现有的标准配置结构,也就是 auth.OPENAI_API_KEYconfig TOML 里的 base_url,这会导致 Codex 在“测试用量脚本”时如果把凭证输入框留空,仍然解析成空字符串,和本 PR “回退到 provider 凭证”的目标不一致; 第二,helper 也丢掉了仓库其他路径已经兼容的 api_key / base_url 别名,因此一部分旧配置或 additive config 依然无法走到 fallback。除此之外,这个 PR 还引入了几处需要在描述里明确说明的行为变化:test_usage_script 现在从“仅使用传入参数”变成“查库后与 provider 配置合并”,凭证优先级也从原先偏向 env.* 变成顶层 apiKey / options.apiKey 优先,同时顶层和 options 里的 baseUrl 现在也会统一去掉尾部 /;这些不一定是问题,但都属于用户可感知的兼容性变化。 另有一个小的实现建议是 test_usage_script 可以直接使用现有的 get_provider_by_id,避免先加载整个 app 下的所有 provider。

感谢review

这次调整:

  • 补了 Codex 现有标准配置结构的 fallback:
    • auth.OPENAI_API_KEY
    • config TOML 里的 base_url
  • 补上了旧别名支持:
    • api_key
    • base_url
    • 以及 options 下对应别名
  • test_usage_script 现在改成直接使用 get_provider_by_id

另外提到的行为变化这部分:

  • test_usage_script 现在确实从“仅使用传入参数”变成了“传入参数为空时回退到 provider 配置”,这一点是这次修复本身需要引入的行为变化
  • 凭证优先级现在会优先走对应 app 已支持的 provider 配置结构,再回退到 env.*
  • 之前这版里额外引入的 baseUrl 去尾 / 我已经去掉了,尽量减少这次 PR 额外带来的兼容性变化

已执行:

cargo fmt --manifest-path src-tauri/Cargo.toml
cargo test resolve_usage_credentials --manifest-path src-tauri/Cargo.toml -- --nocapture
cargo test usage --manifest-path src-tauri/Cargo.toml -- --nocapture

———

补充

usage.rsprovider/mod.rs之前各自维护了一套相近的提取规则,把这部分提取逻辑提取成了共享 helper

  • usage.rs 还是负责 override + fallback
  • provider/mod.rs 还是负责严格提取和报错

@Alexlangl Alexlangl force-pushed the fix/provider-usage-1658 branch from b4788cd to 4cc9ec9 Compare March 29, 2026 08:12
Copy link
Copy Markdown
Owner

@farion1231 farion1231 left a comment

Choose a reason for hiding this comment

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

感谢您的跟进,请查看一下是否存在以下问题:

[P1] Codex 的 base_url fallback 解析方式有误,可能选到错误的 TOML 条目,见 usage.rs (line 93) 和 usage.rs (line 154)。
当前实现只是正则抓取 config 里的第一处 base_url,但 Codex 实际上支持通过 model_provider 选择当前激活的 [model_providers.].base_url,仓库里现有逻辑也是按这个约定处理的,见 codex_config.rs (line 143) 和 providerConfigUtils.ts (line 601)。这会导致多 provider 或存在 mcp_servers.*.base_url 时,请求打到错误 host。

@Alexlangl
Copy link
Copy Markdown
Contributor Author

Alexlangl commented Mar 31, 2026

感谢您的跟进,请查看一下是否存在以下问题:

[P1] Codex 的 base_url fallback 解析方式有误,可能选到错误的 TOML 条目,见 usage.rs (line 93) 和 usage.rs (line 154)。 当前实现只是正则抓取 config 里的第一处 base_url,但 Codex 实际上支持通过 model_provider 选择当前激活的 [model_providers.].base_url,仓库里现有逻辑也是按这个约定处理的,见 codex_config.rs (line 143) 和 providerConfigUtils.ts (line 601)。这会导致多 provider 或存在 mcp_servers.*.base_url 时,请求打到错误 host。

  • 当存在 model_provider 时,优先读取 [model_providers.<current>].base_url
  • 当不存在 model_provider 时,再回退到 top-level base_url
    这样与仓库中现有的 Codex 处理逻辑保持一致

@farion1231

@Alexlangl Alexlangl force-pushed the fix/provider-usage-1658 branch from f46cb7a to 72cf3ac Compare April 3, 2026 06:28
@Alexlangl
Copy link
Copy Markdown
Contributor Author

Closing this for now. I’ve already put a fair amount of time into it, and I don’t want to keep investing more without knowing whether it’s something you still want to move forward.

If you’d like to revisit it later, feel free to let me know.

@Alexlangl Alexlangl closed this Apr 6, 2026
@farion1231
Copy link
Copy Markdown
Owner

Same message as in #1824 — these changes are welcome and I'd love to get them merged. Happy to help rebase if needed.

@Alexlangl Alexlangl reopened this Apr 7, 2026
@Alexlangl
Copy link
Copy Markdown
Contributor Author

Closing this PR and the related ones as I won’t have time to continue working on them.

Thanks for the feedback.

@Alexlangl Alexlangl closed this Apr 22, 2026
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:{{apiKey}}留空时不会自动使用供应商配置中的key

2 participants