Codex capacity - #6658
Conversation
WalkthroughThe PR adds production Docker Compose configuration and environment defaults. It also normalizes OpenAI ChangesProduction deployment configuration
OpenAI overload error normalization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant OpenAI
participant ResponsesRelay
participant ErrorNormalizer
participant Client
OpenAI->>ResponsesRelay: Send overloaded error
ResponsesRelay->>ErrorNormalizer: Normalize server_is_overloaded
ErrorNormalizer-->>ResponsesRelay: Return server_error
ResponsesRelay-->>Client: Return normalized error
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
docker-compose-prd.yaml (2)
10-10: 🩺 Stability & Availability | 🔵 TrivialVerify whether this stack must scale on one host.
Fixed
container_namevalues prevent Docker Compose from creating multiple replicas and can cause collisions with another stack on the same host. If multi-instance deployment is required, remove these fields and use the service names for networking.Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose-prd.yaml` at line 10, Remove the fixed container_name settings from the affected services in the Docker Compose stack, including the new-api service, so Compose can scale replicas and avoid naming collisions; keep service names unchanged for networking.
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the production images to tested immutable references.
calciumion/new-api:latestandredis:latestcan resolve to different content after a restart. This prevents reproducible rollback and can deploy untested changes. Pin each image to a tested version and digest.Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose-prd.yaml` at line 9, Update the production image references in the Docker Compose configuration, including the API image and Redis image, to tested immutable version-and-digest references instead of the floating latest tags. Preserve the existing service configuration while ensuring both deployments and rollbacks resolve to the exact tested image contents..env.example (1)
10-10: 🗄️ Data Integrity & Integration | 🔵 TrivialRequire unique node identities for multi-instance deployments.
common/node_identity.gouses a non-emptyNODE_NAMEverbatim. If multiple production instances reuse this template, all audit records usenew-api-node-1, so the node source is ambiguous. Require a unique value per instance or omit the fallback and use the container hostname.
.env.example#L10-L10: make the sample value an explicit per-instance setting.docker-compose-prd.yaml#L26-L26: remove the fixed fallback or require a unique deployment value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example at line 10, The NODE_NAME configuration currently permits identical node identities across production instances. Update .env.example line 10 to use an explicit per-instance placeholder/value, and update docker-compose-prd.yaml line 26 to remove the fixed fallback or require a unique deployment-provided value; preserve the existing common/node_identity.go behavior while ensuring each instance supplies a distinct identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker-compose-prd.yaml`:
- Line 22: Update the Redis connection configuration around REDIS_CONN_STRING so
REDIS_PASSWORD is URL-encoded before being embedded in the URI, or enforce and
validate a URL-safe password alphabet; ensure passwords containing reserved URL
characters cannot alter parsing or prevent startup.
In `@service/error_test.go`:
- Around line 136-143: Extend the test around NormalizeServerOverloadError and
types.WithOpenAIError to assert that normalizedError preserves
upstreamError.StatusCode. Keep the existing error-code, message, and rewrite
assertions unchanged.
---
Nitpick comments:
In @.env.example:
- Line 10: The NODE_NAME configuration currently permits identical node
identities across production instances. Update .env.example line 10 to use an
explicit per-instance placeholder/value, and update docker-compose-prd.yaml line
26 to remove the fixed fallback or require a unique deployment-provided value;
preserve the existing common/node_identity.go behavior while ensuring each
instance supplies a distinct identity.
In `@docker-compose-prd.yaml`:
- Line 10: Remove the fixed container_name settings from the affected services
in the Docker Compose stack, including the new-api service, so Compose can scale
replicas and avoid naming collisions; keep service names unchanged for
networking.
- Line 9: Update the production image references in the Docker Compose
configuration, including the API image and Redis image, to tested immutable
version-and-digest references instead of the floating latest tags. Preserve the
existing service configuration while ensuring both deployments and rollbacks
resolve to the exact tested image contents.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d7b4fd2-9ce1-4e04-a220-971a89c5ef53
📒 Files selected for processing (9)
.env.exampledocker-compose-prd.yamldocker-compose.ymlrelay/channel/openai/helper.gorelay/channel/openai/relay_responses.gorelay/channel/openai/relay_responses_billing_test.gorelay/responses_handler.goservice/error.goservice/error_test.go
| - ./logs:/app/logs | ||
| environment: | ||
| SQL_DSN: ${SQL_DSN:?SQL_DSN is required} | ||
| REDIS_CONN_STRING: redis://:${REDIS_PASSWORD:?REDIS_PASSWORD is required}@redis:6379 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files around docker-compose-prd.yaml:"
if [ -f docker-compose-prd.yaml ]; then
wc -l docker-compose-prd.yaml
cat -n docker-compose-prd.yaml | sed -n '1,80p'
else
fd -a 'docker-compose-prd\.ya?ml$' .
fi
echo
echo "REDIS_CONN_STRING references:"
rg -n "REDIS_CONN_STRING|redis\.ParseURL|redis://" . -g '!vendor/**' -g '!node_modules/**' | head -200
echo
echo "Go redis parser probe (uses go package data at runtime only if tooling is available):"
cat > /tmp/redis_url_probe.go <<'GO'
package main
import (
"fmt"
neturl "net/url"
)
func main() {
passwords := []string{
"ok",
"p@ss",
"pa/ss",
"pa:ss",
"pa?ss",
"pa#ss",
"pa%ss",
}
for _, pw := range passwords {
raw := fmt.Sprintf("redis://:%s@redis:6379", pw)
u, err := neturl.Parse(raw)
var out string
if err == nil {
var p string
if u.User != nil { p, ok := u.User.Password(); ok }
out = fmt.Sprintf("%s => host=%q user=%q pass=%q path=%q query=%q",
raw, u.Host,
u.User.Username(), p, u.Path, u.Query())
} else {
out = fmt.Sprintf("%s => error=%q", raw, err)
}
fmt.Println(out)
}
}
GO
go run /tmp/redis_url_probe.go 2>/tmp/redis_probe_stderr || { echo "go run failed:"; cat /tmp/redis_probe_stderr; }
rm -f /tmp/redis_url_probe.goRepository: QuantumNous/new-api
Length of output: 4318
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "common/redis.go relevant section:"
cat -n common/redis.go | sed -n '1,120p'
echo
echo "Go URL parser probe with explicit password output:"
cat > /tmp/redis_url_probe.go <<'GO'
package main
import (
"fmt"
neturl "net/url"
)
func main() {
passwords := []string{
"ok",
"p@ss",
"pa/ss",
"pa:ss",
"pa?ss",
"pa#ss",
"pa%ss",
}
for _, pw := range passwords {
raw := fmt.Sprintf("redis://:%s@redis:6379", pw)
u, err := neturl.Parse(raw)
var out string
if err == nil {
var pass string
var havePass bool
if u.User != nil {
pass, havePass = u.User.Password()
}
out = fmt.Sprintf("password=%q raw=%s => host=%q user=%q pass=%q path=%q query=%q", pw, raw, u.Host, u.User.Username(), pass, u.Path, u.Query())
} else {
out = fmt.Sprintf("password=%q raw=%s => error=%q", pw, raw, err)
}
fmt.Println(out)
}
}
GO
go run /tmp/redis_url_probe.go 2>/tmp/redis_probe_stderr || { echo "go run failed:"; cat /tmp/redis_probe_stderr; }
rm -f /tmp/redis_url_probe.goRepository: QuantumNous/new-api
Length of output: 4137
Do not interpolate a raw password into the Redis URI.
REDIS_PASSWORD is embedded directly into REDIS_CONN_STRING before redis.ParseURL parses it. Passwords containing @, :, /, ?, #, or raw % can change the authority or make startup fail. Pass a separately URL-encoded REDIS_CONN_STRING, or enforce a URL-safe password alphabet.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker-compose-prd.yaml` at line 22, Update the Redis connection
configuration around REDIS_CONN_STRING so REDIS_PASSWORD is URL-encoded before
being embedded in the URI, or enforce and validate a URL-safe password alphabet;
ensure passwords containing reserved URL characters cannot alter parsing or
prevent startup.
| openAIError := upstreamError.ToOpenAIError() | ||
| rewritten := NormalizeServerOverloadError(&openAIError) | ||
| normalizedError := types.WithOpenAIError(openAIError, upstreamError.StatusCode) | ||
|
|
||
| require.True(t, rewritten) | ||
| require.Equal(t, types.ErrorCode("server_error"), normalizedError.GetErrorCode()) | ||
| require.Equal(t, "server_error", normalizedError.ToOpenAIError().Code) | ||
| require.Equal(t, "Selected model is at capacity.", normalizedError.Error()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the preserved HTTP status.
Line 138 passes upstreamError.StatusCode into types.WithOpenAIError, but the test does not verify the resulting status. A regression that changes the status during rewrapping will pass this test.
Proposed test assertion
require.True(t, rewritten)
+require.Equal(t, http.StatusServiceUnavailable, normalizedError.StatusCode)
require.Equal(t, types.ErrorCode("server_error"), normalizedError.GetErrorCode())As per coding guidelines, “Backend tests must protect real behavior, API contracts, billing/accounting invariants, compatibility, or regression paths.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| openAIError := upstreamError.ToOpenAIError() | |
| rewritten := NormalizeServerOverloadError(&openAIError) | |
| normalizedError := types.WithOpenAIError(openAIError, upstreamError.StatusCode) | |
| require.True(t, rewritten) | |
| require.Equal(t, types.ErrorCode("server_error"), normalizedError.GetErrorCode()) | |
| require.Equal(t, "server_error", normalizedError.ToOpenAIError().Code) | |
| require.Equal(t, "Selected model is at capacity.", normalizedError.Error()) | |
| openAIError := upstreamError.ToOpenAIError() | |
| rewritten := NormalizeServerOverloadError(&openAIError) | |
| normalizedError := types.WithOpenAIError(openAIError, upstreamError.StatusCode) | |
| require.True(t, rewritten) | |
| require.Equal(t, http.StatusServiceUnavailable, normalizedError.StatusCode) | |
| require.Equal(t, types.ErrorCode("server_error"), normalizedError.GetErrorCode()) | |
| require.Equal(t, "server_error", normalizedError.ToOpenAIError().Code) | |
| require.Equal(t, "Selected model is at capacity.", normalizedError.Error()) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@service/error_test.go` around lines 136 - 143, Extend the test around
NormalizeServerOverloadError and types.WithOpenAIError to assert that
normalizedError preserves upstreamError.StatusCode. Keep the existing
error-code, message, and rewrite assertions unchanged.
Source: Coding guidelines
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
New Features
Bug Fixes
Configuration