Skip to content

feat(config): validate provider existence before auxiliary fields - #1093

Merged
lizhengfeng101 merged 2 commits into
alibaba:mainfrom
Qiyuanqiii:codex/issue-1092-validate-provider-aux-fields
Aug 28, 2026
Merged

feat(config): validate provider existence before auxiliary fields#1093
lizhengfeng101 merged 2 commits into
alibaba:mainfrom
Qiyuanqiii:codex/issue-1092-validate-provider-aux-fields

Conversation

@Qiyuanqiii

@Qiyuanqiii Qiyuanqiii commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

  • Reject extra_body, extra_headers, and retry_codes when a custom provider has not been configured yet.
  • Preserve automatic creation for core provider fields and keep preset-provider configuration unchanged.
  • Reject custom_providers.<preset-name> entries because runtime resolution always treats those names as presets, and point users to the canonical providers.* path or a different custom name.
  • Recommend configuring protocol first so the remediation works for URL-based providers and URL-less custom Bedrock providers.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • make test passes locally
  • Manual testing (describe below)

Additional validation:

  • make check
  • make coverage (90.9% total)
  • ocr review --audience agent (0 findings after the review follow-up)

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (not applicable; no user-facing documentation change is needed)
  • I have signed the CLA

Related Issues

Closes #1092

@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

@Qiyuanqiii Qiyuanqiii left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

我认为当前错误提示仍有两个需要处理的边界问题:

  1. 新测试将 custom_providers.openai.url 作为同名冲突场景的修复方式,但运行时解析会先通过 LookupProvider 判断名称;只要名称命中内置 preset,便只读取 providers.openai,不会读取 custom_providers.openai。这样提示创建的自定义配置实际上无法被选中。这里是否应该直接拒绝与 preset 同名的自定义 provider,并提示用户换名,而不是引导其创建一个不会生效的配置项?

  2. 错误信息固定推荐执行 .url <endpoint>,但受支持的自定义 anthropic-bedrock provider 明确不需要 URL,应该先配置 protocol 等核心字段。是否可以将修复建议改成对所有协议都成立的表述,或者根据 provider 类型提供对应示例?

除此之外,存在性检查的位置、失败时不写入配置以及新增测试整体上没有发现其他问题。

@Qiyuanqiii

Copy link
Copy Markdown
Contributor Author

已处理这两个边界问题,更新在 6f22039

  1. custom_providers.<preset-name> 现在会直接报错,并指向规范化后的 providers.<preset-name>.<field>,同时提示改用不同的自定义名称;测试覆盖了 OpenAI 这类大小写变体。
  2. 缺失 provider 的修复示例改为先设置所有自定义 provider 都必需的 protocol,不再假设存在 URL;现有-provider 成功场景改为自定义 anthropic-bedrock,并断言 URL 保持为空。

本地已重新通过 make checkmake testmake coverage(90.9%)和 ocr review --audience agent(0 findings)。

@Qiyuanqiii
Qiyuanqiii marked this pull request as ready for review August 27, 2026 15:59

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@lizhengfeng101
lizhengfeng101 merged commit da7e0e8 into alibaba:main Aug 28, 2026
12 checks passed
@Qiyuanqiii
Qiyuanqiii deleted the codex/issue-1092-validate-provider-aux-fields branch August 28, 2026 03:56
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.

feat(config): validate provider existence before setting auxiliary fields

2 participants