Skip to content

feat: improve proxy auth compatibility and request logging#74

Open
yuwei5380 wants to merge 3 commits intoicebear0828:masterfrom
yuwei5380:master
Open

feat: improve proxy auth compatibility and request logging#74
yuwei5380 wants to merge 3 commits intoicebear0828:masterfrom
yuwei5380:master

Conversation

@yuwei5380
Copy link

1、support openclaw、opencode: set default instructions(You are a helpful assistant.)
2、record llm logging
3、add gpt 5.3、gpt5.4
4、support openclaw: fix header auth

Copy link
Owner

@icebear0828 icebear0828 left a comment

Choose a reason for hiding this comment

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

感谢提交!Auth 兼容和 admin basic auth 的思路很好,但这个 PR 混合了多个不相关的改动,且有几个阻断问题需要先解决:

需要 Revert 的改动

config/default.yaml

  • 硬编码了 proxy_api_keyadmin_basic_auth_password,密钥会永久留在 git 历史中,请务必 revert 这个文件的所有改动
  • 默认端口 8080→18080、模型改 gpt-5.4、reasoning 改 xhigh、host 从 ::0.0.0.0(丢失 IPv6)——这些是个人偏好,不应改动上游默认值

docker-compose.yml

  • 镜像地址改为 yuwei/codex-proxy:latest,这是私人镜像,需要 revert

config/models.yaml

  • 新模型(gpt-5.4、gpt-5.3-codex)已通过后端动态获取自动同步到 models.yaml,不需要手动添加,请 revert

需要讨论的改动

src/routes/responses.ts — instructions 默认值

  • instructions 从必填改为可选并注入 "You are a helpful assistant." —— Codex API 要求 instructions 为必填字段,注入默认值可能导致上游行为不可预测。建议改为在文档中说明客户端需要配置 instructions

src/proxy/codex-api.ts — interactionContext

  • mutable setter setInteractionContext 在 retry 场景下可能被多次覆盖,建议改为将 logger 作为 createResponse 的参数传入

可以接受的部分 👍

  • proxy-auth.ts — 统一 API key 提取逻辑,支持 Authorization/x-api-key/api-key 多种格式,兼容 openclaw/opencode
  • admin-basic-auth.ts — 管理端 Basic Auth 中间件,用了 timingSafeEqual
  • llm-interaction-log.ts — 结构化请求日志系统(建议补充测试)

建议

拆分为 2-3 个独立 PR 会更容易 review 和合并:

  1. proxy auth 兼容proxy-auth.ts + 各路由的 key 提取重构
  2. admin basic auth — 中间件 + config schema + 测试
  3. LLM interaction log — logger + proxy-handler 集成 + 测试

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