feat(ws): add OpenAI Responses WebSocket support#887
Open
Conversation
…transport classifier Wave 1 of OpenAI Responses WebSocket support: - Add enableResponsesWebSocket system setting across all layers (schema, types, repository, cache, actions, UI, 5 locales) mirroring enableHttp2 - Create WS frame Zod schemas (response.create, terminal events, errors) with forward-compatible .passthrough() and byte-for-byte encrypted_content - Add same-port custom Node server (src/server/) wrapping Next.js + ws library for /v1/responses WebSocket upgrades with heartbeat/graceful shutdown - Add transport classifier deciding HTTP vs WebSocket based on toggle, endpoint, provider type, URL scheme, and proxy config - Add ws_fallback provider chain reason mirroring http2_fallback semantics (no circuit breaker penalty, no provider switch)
…and session continuity Wave 2 of OpenAI Responses WebSocket support: - OutboundWsAdapter: request-scoped upstream WS connection with configurable timeouts, flex tier support, and terminal event collection - WsIngressHandler: delayed bridging state machine (waiting/processing/closed) with auth validation, first-frame timeout, and sequential turn support - WsEventBridge: bounded ring buffer for event relay with terminal settlement (response.completed/failed/incomplete) and usage extraction - Session continuity: turn context tracking, prompt_cache_key session binding, disconnect classification (retryable vs terminal), neutral fallback rules
…verage Wave 3 of OpenAI Responses WebSocket support: - Billing parity: WS terminal events feed through same cost/usage pipeline as HTTP (settleWsTurnBilling, wsUsageToMetrics, redactWsEventPayload) - Provider testing: WS probe backend (probeProviderWebSocket) reports transport, handshake latency, event count, fallback reason - Provider testing UI: WsTestStatus component with transport badge, metrics display, and 5-language i18n - Trace metadata builder for Langfuse with WS transport fields
📝 WalkthroughWalkthrough本次变更在项目中引入完整的 WebSocket 路径：架构定义、服务器与管理器、入站/出站适配器、传输分类、会话连续性、账单对齐、帧模式与解析、设置开关与本地化，以及大量相关单元测试和数据库迁移（新增 enableResponsesWebSocket）。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Claude Code Hub by integrating WebSocket support for OpenAI's Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
enhancement
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive support for WebSocket transport for OpenAI /v1/responses. The changes are well-structured across multiple modules, covering the full lifecycle from connection handling and authentication to billing and testing parity. The implementation is robust, with detailed error handling, state management, and extensive test coverage. I have one suggestion to improve the WebSocket URL construction logic to make it more resilient to different provider base URL formats.
Note: Security Review did not run due to the size of the PR.
Comment on lines
+66
to
+74
| export function toWebSocketUrl(providerBaseUrl: string): string { | ||
| const url = new URL(providerBaseUrl); | ||
| url.protocol = "wss:"; | ||
| // Ensure path ends with /v1/responses | ||
| if (!url.pathname.endsWith("/v1/responses")) { | ||
| url.pathname = url.pathname.replace(/\/$/, "") + "/v1/responses"; | ||
| } | ||
| return url.toString(); | ||
| } |
Contributor
There was a problem hiding this comment.
The current implementation for constructing the WebSocket URL might lead to incorrect paths if the provider's base URL already contains path segments. For example, if providerBaseUrl is https://example.com/v1, the resulting URL would be wss://example.com/v1/v1/responses, which is likely not intended.
To make this more robust and align with the apparent intent of always targeting the /v1/responses endpoint on the given host, consider directly setting the pathname.
export function toWebSocketUrl(providerBaseUrl: string): string {
const url = new URL(providerBaseUrl);
url.protocol = "wss:";
url.pathname = "/v1/responses";
return url.toString();
}![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+66
to
+74
| export function toWebSocketUrl(providerBaseUrl: string): string { | ||
| const url = new URL(providerBaseUrl); | ||
| url.protocol = "wss:"; | ||
| // Ensure path ends with /v1/responses | ||
| if (!url.pathname.endsWith("/v1/responses")) { | ||
| url.pathname = url.pathname.replace(/\/$/, "") + "/v1/responses"; | ||
| } | ||
| return url.toString(); | ||
| } |
There was a problem hiding this comment.
Path double-append bug when base URL includes /v1
toWebSocketUrl appends /v1/responses to the URL's existing pathname without clearing it first. When a provider is configured with the common base URL pattern https://api.openai.com/v1, the resulting WebSocket URL becomes wss://api.openai.com/v1/v1/responses instead of the correct wss://api.openai.com/v1/responses.
Example:
Input: https://api.openai.com/v1
Output: wss://api.openai.com/v1/v1/responses ← wrong
Wanted: wss://api.openai.com/v1/responses
The fix is to reset the pathname to the target path:
Suggested change
| export function toWebSocketUrl(providerBaseUrl: string): string { | |
| const url = new URL(providerBaseUrl); | |
| url.protocol = "wss:"; | |
| // Ensure path ends with /v1/responses | |
| if (!url.pathname.endsWith("/v1/responses")) { | |
| url.pathname = url.pathname.replace(/\/$/, "") + "/v1/responses"; | |
| } | |
| return url.toString(); | |
| } | |
| export function toWebSocketUrl(providerBaseUrl: string): string { | |
| const url = new URL(providerBaseUrl); | |
| url.protocol = "wss:"; | |
| url.pathname = "/v1/responses"; | |
| return url.toString(); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/transport-classifier.ts
Line: 66-74
Comment:
**Path double-append bug when base URL includes `/v1`**
`toWebSocketUrl` appends `/v1/responses` to the URL's existing pathname without clearing it first. When a provider is configured with the common base URL pattern `https://api.openai.com/v1`, the resulting WebSocket URL becomes `wss://api.openai.com/v1/v1/responses` instead of the correct `wss://api.openai.com/v1/responses`.
Example:
```
Input: https://api.openai.com/v1
Output: wss://api.openai.com/v1/v1/responses ← wrong
Wanted: wss://api.openai.com/v1/responses
```
The fix is to reset the pathname to the target path:
```suggestion
export function toWebSocketUrl(providerBaseUrl: string): string {
const url = new URL(providerBaseUrl);
url.protocol = "wss:";
url.pathname = "/v1/responses";
return url.toString();
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+24
to
+40
| // Placeholder connection handler (will be replaced by ingress-handler in T6) | ||
| wsManager.onConnection((ws, req) => { | ||
| const url = new URL(req.url || "/", `http://${req.headers.host || "localhost"}`); | ||
| console.log(`[WS] New connection on ${url.pathname}`); | ||
|
|
||
| ws.on("message", () => { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: { | ||
| type: "server_error", | ||
| message: "WebSocket ingress not yet initialized", | ||
| }, | ||
| }) | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Placeholder handler is never replaced with registerIngressHandler
src/server/index.ts registers a placeholder onConnection handler but never calls registerIngressHandler from src/app/v1/_lib/ws/ingress-handler.ts (line 352-363). The WsIngressHandler class implements auth validation, toggle checking, and connection state management, but the real handler is never integrated. The placeholder currently returns "WebSocket ingress not yet initialized" to all messages, bypassing the auth and toggle checks that WsIngressHandler.start() would perform.
Replace the placeholder handler with a call to registerIngressHandler:
Suggested change
| // Placeholder connection handler (will be replaced by ingress-handler in T6) | |
| wsManager.onConnection((ws, req) => { | |
| const url = new URL(req.url || "/", `http://${req.headers.host || "localhost"}`); | |
| console.log(`[WS] New connection on ${url.pathname}`); | |
| ws.on("message", () => { | |
| ws.send( | |
| JSON.stringify({ | |
| type: "error", | |
| error: { | |
| type: "server_error", | |
| message: "WebSocket ingress not yet initialized", | |
| }, | |
| }) | |
| ); | |
| }); | |
| }); | |
| import { registerIngressHandler } from "@/app/v1/_lib/ws/ingress-handler"; | |
| // ... after creating wsManager ... | |
| registerIngressHandler(wsManager); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/index.ts
Line: 24-40
Comment:
**Placeholder handler is never replaced with `registerIngressHandler`**
`src/server/index.ts` registers a placeholder `onConnection` handler but never calls `registerIngressHandler` from `src/app/v1/_lib/ws/ingress-handler.ts` (line 352-363). The `WsIngressHandler` class implements auth validation, toggle checking, and connection state management, but the real handler is never integrated. The placeholder currently returns `"WebSocket ingress not yet initialized"` to all messages, bypassing the auth and toggle checks that `WsIngressHandler.start()` would perform.
Replace the placeholder handler with a call to `registerIngressHandler`:
```suggestion
import { registerIngressHandler } from "@/app/v1/_lib/ws/ingress-handler";
// ... after creating wsManager ...
registerIngressHandler(wsManager);
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+328
to
+342
| function extractClientIp(req: IncomingMessage): string { | ||
| const realIp = req.headers["x-real-ip"]; | ||
| if (typeof realIp === "string" && realIp.trim()) return realIp.trim(); | ||
|
|
||
| const forwarded = req.headers["x-forwarded-for"]; | ||
| if (typeof forwarded === "string") { | ||
| const ips = forwarded | ||
| .split(",") | ||
| .map((s) => s.trim()) | ||
| .filter(Boolean); | ||
| if (ips.length > 0) return ips[ips.length - 1]; | ||
| } | ||
|
|
||
| return req.socket?.remoteAddress ?? "unknown"; | ||
| } |
There was a problem hiding this comment.
x-forwarded-for takes last hop instead of first
if (ips.length > 0) return ips[ips.length - 1];The x-forwarded-for header is structured as client, proxy1, proxy2, …. The first entry is the originating client IP. The last entry is the most recently appended proxy, which can be forged if the ingress doesn't validate the header. This should use the first entry for correct client IP identification:
Suggested change
| function extractClientIp(req: IncomingMessage): string { | |
| const realIp = req.headers["x-real-ip"]; | |
| if (typeof realIp === "string" && realIp.trim()) return realIp.trim(); | |
| const forwarded = req.headers["x-forwarded-for"]; | |
| if (typeof forwarded === "string") { | |
| const ips = forwarded | |
| .split(",") | |
| .map((s) => s.trim()) | |
| .filter(Boolean); | |
| if (ips.length > 0) return ips[ips.length - 1]; | |
| } | |
| return req.socket?.remoteAddress ?? "unknown"; | |
| } | |
| if (ips.length > 0) return ips[0]; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/ws/ingress-handler.ts
Line: 328-342
Comment:
**`x-forwarded-for` takes last hop instead of first**
```ts
if (ips.length > 0) return ips[ips.length - 1];
```
The `x-forwarded-for` header is structured as `client, proxy1, proxy2, …`. The **first** entry is the originating client IP. The last entry is the most recently appended proxy, which can be forged if the ingress doesn't validate the header. This should use the first entry for correct client IP identification:
```suggestion
if (ips.length > 0) return ips[0];
```
How can I resolve this? If you propose a fix, please make it concise.![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR introduces comprehensive WebSocket infrastructure for OpenAI /v1/responses (~10,500 lines across 59 files). While the architecture is well-structured with thorough test coverage (241 WS-specific tests), three critical integration bugs prevent the feature from functioning and must be fixed before merge.
PR Size: XL
- Lines changed: 10,584 additions, 5 deletions
- Files changed: 59
- Recommendation: This PR is very large. Consider splitting future work: Wave 1 (foundation/toggle), Wave 2 (core pipeline), Wave 3 (parity/UI) could be separate sequential PRs.
Critical Issues (Must Fix)
| Category | File | Issue | Impact |
|---|---|---|---|
| Logic/Bug | toWebSocketUrl double-appends /v1/responses causing paths like /v1/v1/responses | All WS connections to standard /v1 providers fail with 404 | |
| Logic/Bug | X-Forwarded-For takes LAST ip instead of FIRST | Client IP logging, rate limiting use proxy IP instead of actual client | |
| Standard-Violation | Placeholder handler never replaced; registerIngressHandler never called | WS feature is non-functional - all connections receive placeholder error |
High Priority Issues (Should Fix)
| Category | File | Issue |
|---|---|---|
| Error-Handling | Non-JSON messages from server are silently dropped with only a warn log | |
| Type-Design | isNeutralFallback uses 'message?.includes()' on potentially undefined message |
Testing
- 241 WS-specific tests across 12 test files - excellent coverage
- All 3,775 tests passing
- Pre-commit checklist verified (typecheck, lint, test, build)
Review Coverage
- Logic and correctness - 3 critical bugs found
- Security (OWASP Top 10) - IP spoofing risk from X-Forwarded-For bug
- Error handling - Generally good, some silent failures
- Type safety - Good use of Zod schemas
- Documentation accuracy - Good comments and JSDoc
- Test coverage - Excellent (80%+ required, 241 WS tests)
- Code clarity - Well-structured architecture
Automated review by Claude AI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[locale]/settings/config/_components/system-settings-form.tsx (1)
199-225:⚠️ Potential issue | 🟡 Minor保存后未从服务器响应更新
enableResponsesWebSocket状态。其他设置字段（如
enableHttp2、verboseProviderError等）在保存成功后会从result.data更新本地状态，但enableResponsesWebSocket缺失了这一步。如果服务器端对该值有任何规范化处理，UI 状态可能与实际保存的值不一致。建议添加状态更新
setEnableHttp2(result.data.enableHttp2); + setEnableResponsesWebSocket(result.data.enableResponsesWebSocket); setInterceptAnthropicWarmupRequests(result.data.interceptAnthropicWarmupRequests);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/config/_components/system-settings-form.tsx around lines 199 - 225, The save-result handling block updates many local state setters but omits updating enableResponsesWebSocket, which can leave the UI out of sync with server-normalized values; add a call to setEnableResponsesWebSocket(result.data.enableResponsesWebSocket) inside the if (result.data) { ... } block (near existing setters like setEnableHttp2 and setVerboseProviderError) so the local state reflects the server response after saving.
🟡 Minor comments (2)
src/app/v1/_lib/proxy/transport-classifier.ts-66-74 (1)
66-74:⚠️ Potential issue | 🟡 Minor
toWebSocketUrl在特定边界情况下可能产生冗余路径当输入 URL 已包含
/v1/responses/（带尾部斜杠）时，检查!url.pathname.endsWith("/v1/responses")为 true，会导致追加额外的/v1/responses。例如：
https://api.com/v1/responses/→wss://api.com/v1/responses/v1/responses建议的修复
export function toWebSocketUrl(providerBaseUrl: string): string { const url = new URL(providerBaseUrl); url.protocol = "wss:"; // Ensure path ends with /v1/responses - if (!url.pathname.endsWith("/v1/responses")) { + const normalizedPath = url.pathname.replace(/\/+$/, ""); + if (!normalizedPath.endsWith("/v1/responses")) { - url.pathname = url.pathname.replace(/\/$/, "") + "/v1/responses"; + url.pathname = normalizedPath + "/v1/responses"; } return url.toString(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/transport-classifier.ts` around lines 66 - 74, toWebSocketUrl currently appends "/v1/responses" even when the incoming pathname already ends with "/v1/responses/" which creates duplicate segments; inside toWebSocketUrl normalize url.pathname by trimming any trailing slash (or otherwise normalizing) before checking endsWith("/v1/responses"), and only set url.pathname = (trimmedPath === "" ? "/v1/responses" : trimmedPath + "/v1/responses") when the trimmed path does not already end with "/v1/responses", ensuring inputs like "/v1/responses/" become "/v1/responses" and avoiding duplicate "/v1/responses/v1/responses".src/server/index.ts-4-4 (1)
4-4:⚠️ Potential issue | 🟡 Minor统一用
@/引用src内模块。这里直接相对引用
./ws-manager，和仓库里对src内部依赖的路径约定不一致，后续移动文件时也更容易出现路径漂移。建议修改
-import { WsManager } from "./ws-manager"; +import { WsManager } from "@/server/ws-manager";As per coding guidelines, "Use path alias
@/to reference files in./src/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/index.ts` at line 4, The import in src/server/index.ts uses a relative path for WsManager ("./ws-manager"); change it to use the project path-alias by importing WsManager from "@/ws-manager" so it follows the repo convention and avoids future path-drift (update the import statement that references WsManager accordingly).
🧹 Nitpick comments (7)
src/lib/ws/frame-parser.ts (1)
38-42: 路径为空时错误消息格式略有瑕疵当 Zod 校验的
path为空数组时，path.join(".")返回空字符串，导致错误消息格式为": Required"而非"Required"。这是一个细微的展示问题，不影响功能。
可选修复：处理空路径情况
const firstIssue = result.error.issues[0]; - const message = firstIssue - ? `${firstIssue.path.join(".")}: ${firstIssue.message}` + const message = firstIssue + ? firstIssue.path.length > 0 + ? `${firstIssue.path.join(".")}: ${firstIssue.message}` + : firstIssue.message : "Schema validation failed";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/ws/frame-parser.ts` around lines 38 - 42, The error message construction in the frame-parser's validation handling uses firstIssue.path.join(".") which yields an empty string when path is [], producing messages like ": Required"; update the logic that builds message (where result.error.issues[0] / firstIssue is used) to detect an empty path (e.g., firstIssue.path.length === 0) and omit the leading colon/empty prefix so the message becomes just firstIssue.message, otherwise keep `${firstIssue.path.join(".")}: ${firstIssue.message}`.tests/unit/proxy/transport-classifier.test.ts (1)
233-236: 建议添加尾部斜杠边界情况测试当前测试覆盖了
/v1/responses路径保留的情况，但缺少/v1/responses/（带尾部斜杠）的测试用例。根据toWebSocketUrl实现分析，这可能是一个边界情况。it("preserves existing /v1/responses path", () => { const result = toWebSocketUrl("https://api.openai.com/v1/responses"); expect(result).toBe("wss://api.openai.com/v1/responses"); }); + + it("handles /v1/responses with trailing slash", () => { + const result = toWebSocketUrl("https://api.openai.com/v1/responses/"); + expect(result).toBe("wss://api.openai.com/v1/responses"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/proxy/transport-classifier.test.ts` around lines 233 - 236, Add a unit test covering the trailing-slash edge case for toWebSocketUrl: create an it block (similar to the existing "preserves existing /v1/responses path") that calls toWebSocketUrl("https://api.openai.com/v1/responses/") and asserts the returned value preserves the trailing slash and uses wss (e.g., "wss://api.openai.com/v1/responses/"); reference the existing test name and the toWebSocketUrl function so the new test sits alongside the current case and verifies the "/v1/responses/" boundary behavior.src/app/v1/_lib/proxy/transport-classifier.ts (1)
38-42: 端点检查使用endsWith可能过于宽松当前检查
pathname.endsWith("/responses")会匹配/v1/responses，但也会匹配/foo/responses等非预期路径。考虑到代理路由定义明确，这在实际场景中可能不是问题。如需更严格匹配，可考虑：
- if (!pathname.endsWith("/responses")) { + if (pathname !== "/v1/responses" && !pathname.endsWith("/v1/responses")) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/transport-classifier.ts` around lines 38 - 42, The endpoint check in transport-classifier.ts currently uses pathname.endsWith("/responses") which is too permissive; update the logic in the block that reads const pathname = session.requestUrl.pathname so it only accepts the exact proxy route (e.g., compare pathname === "/v1/responses") or validate path segments (e.g., split pathname by "/" and ensure segments match ["", "v1", "responses"] or use a strict RegExp like /^\/v1\/responses(?:\/)?$/) and return the same { transport: "http", reason: "not_responses_endpoint" } for anything that doesn't match.src/app/v1/_lib/ws/outbound-adapter.ts (1)
5-5:src/内部引用请统一走@/别名这里新加了相对路径导入，和仓库的
src引用约定不一致。请改成@/app/v1/_lib/proxy/transport-classifier。As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use path alias@/to reference files in./src/directory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/outbound-adapter.ts` at line 5, Import in outbound-adapter.ts uses a relative path; change the import of toWebSocketUrl to use the project alias by replacing "../proxy/transport-classifier" with "@/app/v1/_lib/proxy/transport-classifier" so the file imports toWebSocketUrl via the `@/` src alias consistent with repository conventions.src/server/ws-manager.ts (1)
73-84: 优雅关闭实现良好。关闭流程正确：先清理心跳定时器，然后用 1001（Going Away）状态码关闭所有客户端连接，最后关闭 WebSocketServer。
可选改进：可以考虑为
wss.close()添加超时保护，防止某些客户端连接关闭缓慢导致阻塞。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/ws-manager.ts` around lines 73 - 84, 当前 close() 返回的 Promise 可能会被某些挂起的客户端关闭操作阻塞；在 close()（使用 heartbeatInterval、this.wss 和 this.wss.close()）里为 this.wss.close() 的异步等待添加超时保护：将现有 new Promise 包装成带超时的 Promise.race，一方面保留 clearInterval(this.heartbeatInterval) 和遍历 this.wss.clients 调用 ws.close(1001,... )，另一方面在超时（例如 5s）触发时调用 this.wss.clients.forEach(ws => ws.terminate()) 或直接调用 this.wss.close() 的强制完成分支并 resolve，以确保 close() 在可控时间内返回。src/app/v1/_lib/ws/session-continuity.ts (1)
4-5: 这里也建议改成@/别名导入。这两个目标都在
src/下，继续走相对路径会让模块边界和移动成本都更差。As per coding guidelines "Use path alias@/to reference files in./src/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/session-continuity.ts` around lines 4 - 5, Replace the relative imports with the project path-alias `@/` so the module uses aliases rather than relative paths: change the import references for the types SettlementResult, TurnMeta, and WsAuthContext in session-continuity.ts to use `@/`-prefixed paths that point into the src tree (e.g., import SettlementResult from '@/...' and import { TurnMeta, WsAuthContext } from '@/...'), ensuring the alias matches your tsconfig/webpack config and that the exported type names (SettlementResult, TurnMeta, WsAuthContext) remain unchanged.src/app/v1/_lib/ws/ingress-handler.ts (1)
12-12: 这里的内部导入建议统一改成@/别名。
src/内再走../proxy/auth-guard，后续挪文件时很容易把路径关系一起打断，也和仓库约定不一致。As per coding guidelines "Use path alias@/to reference files in./src/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/ingress-handler.ts` at line 12, In src/app/v1/_lib/ws/ingress-handler.ts replace the relative import of extractApiKeyFromHeaders from "../proxy/auth-guard" with the project path-alias style using "@/..." (e.g. import extractApiKeyFromHeaders from "@/app/v1/_lib/proxy/auth-guard") so internal imports consistently use the `@/` alias; keep the imported symbol name extractApiKeyFromHeaders unchanged and ensure your tsconfig/paths supports the alias.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@deploy/Dockerfile`:
- Around line 61-63: Update the Dockerfile CMD to run the custom server entry
used for WebSocket handling instead of the default Next.js standalone server:
replace the existing CMD that launches server.js with one that starts your
custom server entry (src/server/index.ts compiled/started appropriately) and
remove or correct the outdated TODO/comment that references src/server/index.js;
ensure the Docker runtime/start command aligns with how you build and run
TypeScript (e.g., use the compiled JS entry or a node wrapper) so the custom
entry that references ws-manager.ts is actually launched.
In `@Dockerfile`:
- Around line 28-30: The Dockerfile currently launches the Next.js standalone
server (CMD running server.js) which skips the custom HTTP+WS bootstrap in
src/server/index.ts; update the Dockerfile to run the custom server entry
instead (start node with tsx importing src/server/index.ts) or alternatively
change the build so that the compiled server.js includes the custom server code
from src/server/index.ts; ensure the container runs the entry that initializes
WsManager/WebSocketServer and mounts the /v1/responses route rather than the
standalone Next output.
In `@package.json`:
- Line 9: package.json 中的 "start:ws" 脚本引用了 node --import tsx 但项目未声明 tsx
依赖，导致在干净安装后启动失败；修复方法是要么在 package.json 的 devDependencies 中添加 "tsx"（并运行 npm/yarn
安装），要么修改 "start:ws" 脚本为直接使用 tsx 可执行命令（例如 "tsx src/server/index.ts"），请更新
package.json 中的 "start:ws" 脚本或 devDependencies 以确保 tsx 在运行时可用。
In `@src/app/`[locale]/settings/providers/_components/forms/ws-test-status.tsx:
- Around line 53-56: Do not render result.wsFallbackReason directly; instead map
that internal reason code to an i18n key and pass the key to t() with a
fallback. Add a small mapping object (e.g., wsFallbackReasonKeyMap) that maps
known reason codes to translation keys like "ws.fallbackReason.network",
"ws.fallbackReason.auth", etc., then compute const reasonKey =
wsFallbackReasonKeyMap[result.wsFallbackReason] ?? "ws.fallbackReason.unknown"
and render t(reasonKey) where result.wsFallbackReason is currently shown;
optionally keep the raw value in a debug log but never expose it in the UI.
Ensure translation keys exist for all five supported locales.
In `@src/app/v1/_lib/ws/billing-parity.ts`:
- Around line 206-225: calculateRequestCost() is called with costMultiplier but
calculateRequestCostBreakdown() is not, causing mismatched total vs breakdown
when a provider multiplier is applied; update the
calculateRequestCostBreakdown(...) call to accept and forward costMultiplier
(same as calculateRequestCost) along with usageMetrics, priceData,
context1mApplied, priorityServiceTierApplied so result.costBreakdown uses the
same multiplier and matches result.costUsd (ensure the
calculateRequestCostBreakdown function signature is updated where defined to
accept the new costMultiplier parameter).
- Around line 145-164: The wsUsageToMetrics function currently only reads
top-level passthrough fields and misses the nested cached tokens fallback;
update wsUsageToMetrics (and its usage of ResponseUsage/raw) to check for
input_tokens_details?.cached_tokens and use that value as
cache_read_input_tokens when raw.cache_read_input_tokens is undefined or not a
number, mirroring the logic in response-handler.ts; add a unit test that
supplies a ResponseUsage with no top-level cache_read_input_tokens but with
input_tokens_details.cached_tokens to ensure the converted UsageMetrics includes
cache_read_input_tokens.
In `@src/app/v1/_lib/ws/ingress-handler.ts`:
- Around line 134-138: The debug log in the WsIngress auth path currently
records raw PII (authResult.user.name and this.ip); change the logger.debug call
(the one that currently uses authResult.user.id, authResult.user.name, this.ip)
to only include userId and a non-reversible or truncated IP representation
(e.g., hash the IP with SHA-256 or mask/truncate it) and remove
authResult.user.name entirely; ensure the new fields are clearly named (e.g.,
userId and clientIpHash or clientIpMask) so callers can identify them when
searching logs.
- Around line 88-142: The start() method can throw when calling
isResponsesWebSocketEnabled() or validateApiKeyAndGetUser() (and likewise at the
other call site around lines 356-359); wrap the async startup sequence in a
try/catch that catches any exception, logs the error (include context like
user/IP), calls this.ws.close(...) with a protocol-appropriate code and message,
sets this.state = "closed", and returns false; ensure the same defensive
try/catch is applied where start() is invoked (e.g., in the onConnection
handler) so no exception bubbles out unhandled and setupListeners() is only
called after successful, non-throwing authentication.
- Around line 269-279: The handleTurn method currently always sends a
server_error and never forwards response.create frames upstream; replace the
unconditional this.sendError with logic that builds a synthetic ProxySession
from this.req and this.auth, runs the deferred guard pipeline (model, rateLimit,
provider) and then attempts to forward the ResponseCreateFrame to the outbound
bridge/adapter (use existing outbound adapter or HTTP fallback), relaying
upstream events back to the client; only call this.sendError if both the bridge
attempt and HTTP fallback fail, and ensure any error includes contextual details
for debugging.
- Around line 332-338: The code handling the "x-forwarded-for" header in
ingress-handler.ts is returning the last IP in the list (ips[ips.length - 1])
which is reversed; change the logic to return the first IP (the
left-most/ips[0]) as the original client IP. In the block that reads const
forwarded = req.headers["x-forwarded-for"] and builds ips, replace the return to
use ips[0] (or otherwise select the first trimmed, non-empty element) so
downstream rate-limiting/audit/provider decisions use the true client IP.
- Line 93: Replace hardcoded English close/reason strings passed to
this.ws.close(...) with stable error codes/constants and ensure the client/UI
performs i18n mapping; specifically change the call using this.ws.close(4003,
"Responses WebSocket is disabled") to send a predefined error identifier (e.g.,
"ERR_WS_RESPONSES_DISABLED" or an exported enum value) and update all other
similar calls (the other this.ws.close(...) occurrences flagged) to use the same
pattern so server only emits codes and the presentation layer handles language
mapping for zh-CN/zh-TW/en/ja/ru.
In `@src/app/v1/_lib/ws/outbound-adapter.ts`:
- Around line 158-234: The issue is that late-arriving WebSocket callbacks
(open/message/error/close) can run after finish() has resolved the request,
causing duplicate sends and mutating events; update each WS event handler (the
"open", "message", "error", and "close" listeners in outbound-adapter.ts) to
immediately short-circuit if the local resolved flag is true, and ensure
finish() both sets resolved and removes/cleans the socket listeners and any
timers (e.g., clearTimeout(handshakeTimer) and remove listeners via ws.off or
ws.removeAllListeners) before attempting close; also add a regression test
asserting that a delayed "open" after a timeout does not call send() and that
late "message" events do not modify the returned events array.
In `@src/app/v1/_lib/ws/session-continuity.ts`:
- Around line 132-146: The logs in SessionContinuity that write promptCacheKey
and sessionId (see logger.debug and logger.error in the session update block)
expose sensitive correlating identifiers; change both debug and error logging to
avoid writing the raw promptCacheKey and full sessionId by instead logging a
presence boolean and a deterministic redaction (e.g., truncated substring) or a
hash of promptCacheKey and sessionId; update the logging calls around the
return/catch in the function handling session binding (references:
promptCacheKey, sessionId, logger.debug, logger.error, result.sessionId,
result.updated) so logs include only non-sensitive indicators (exists:
true/false, masked: "xxxx...abcd" or sha256(promptCacheKey)) and retain other
contextual fields like providerId.
In `@src/lib/ws/frames.ts`:
- Around line 9-15: ReasoningConfigSchema currently restricts reasoning.effort
to ["minimal","low","medium","high"] which rejects valid OpenAI Responses API
values; update the enum in ReasoningConfigSchema (the z.object that defines
effort) to include "none" and "xhigh" so it accepts
["none","minimal","low","medium","high","xhigh"], and ensure any downstream
validation/type checks that reference reasoning.effort are compatible with these
additional values.
- Around line 41-64: The ResponseCreateFrameSchema currently enforces
response.input as an array only; update the schema (ResponseCreateFrameSchema)
so response.input accepts either a string or an array by replacing the input
type with z.union([z.string(), z.array(InputItemSchema)]).optional() (keep
.passthrough() on the parent object) and add unit tests for the parser/validator
to cover both input: "Hello!" and input: ["..."] cases to ensure HTTP/WS
semantics accept single-string input as well as arrays; reference
InputItemSchema when implementing the union so array validation remains
unchanged.
In `@src/repository/system-config.ts`:
- Around line 341-344: The update routine sets payload.enableResponsesWebSocket
into updates but doesn’t include that column in the returning(...) projection,
so toSystemSettings(updated) receives undefined; modify the DB update call to
include enableResponsesWebSocket in the returning(...) list (the same projection
used for other fields) so the updated row returned to toSystemSettings contains
the real value for enableResponsesWebSocket, and verify the
toSystemSettings/updates mapping uses the same property name.
In `@src/server/index.ts`:
- Around line 24-39: The placeholder WebSocket connection handler
(wsManager.onConnection) immediately replies with a server_error and never wires
the real ingress/bridge; replace the stubbed message handler with the actual
WsIngressHandler/pipeline so connections to /v1/responses are forwarded to the
real ingress logic. Locate the wsManager.onConnection callback, instantiate or
retrieve the WsIngressHandler (or existing pipeline) and attach it to the ws
connection and req (handing over message/event handling and close/error
propagation), removing the current hardcoded ws.on("message", ...) that returns
the "WebSocket ingress not yet initialized" error.
- Around line 7-8: The code uses process.env.HOSTNAME for the bind address
(const hostname), which is unsafe in containers; change it to read from
process.env.HOST with a fallback to "0.0.0.0" (i.e., replace references to
process.env.HOSTNAME in the hostname initialization with process.env.HOST ||
"0.0.0.0") so the server binds to a proper network interface instead of a
pod/container name; keep the existing parseInt for port unchanged.
- Around line 43-47: The shutdown function currently calls server.close() and
immediately process.exit(0), which can terminate in-flight HTTP requests; change
shutdown to await server.close by wrapping server.close in a Promise (e.g., new
Promise((resolve, reject) => server.close(err => err ? reject(err) :
resolve()))) before calling process.exit(0), keep awaiting wsManager.close() as
is, and handle errors from the awaited server.close to log and exit with an
appropriate code instead of exiting immediately.
---
Outside diff comments:
In `@src/app/`[locale]/settings/config/_components/system-settings-form.tsx:
- Around line 199-225: The save-result handling block updates many local state
setters but omits updating enableResponsesWebSocket, which can leave the UI out
of sync with server-normalized values; add a call to
setEnableResponsesWebSocket(result.data.enableResponsesWebSocket) inside the if
(result.data) { ... } block (near existing setters like setEnableHttp2 and
setVerboseProviderError) so the local state reflects the server response after
saving.
---
Minor comments:
In `@src/app/v1/_lib/proxy/transport-classifier.ts`:
- Around line 66-74: toWebSocketUrl currently appends "/v1/responses" even when
the incoming pathname already ends with "/v1/responses/" which creates duplicate
segments; inside toWebSocketUrl normalize url.pathname by trimming any trailing
slash (or otherwise normalizing) before checking endsWith("/v1/responses"), and
only set url.pathname = (trimmedPath === "" ? "/v1/responses" : trimmedPath +
"/v1/responses") when the trimmed path does not already end with
"/v1/responses", ensuring inputs like "/v1/responses/" become "/v1/responses"
and avoiding duplicate "/v1/responses/v1/responses".
In `@src/server/index.ts`:
- Line 4: The import in src/server/index.ts uses a relative path for WsManager
("./ws-manager"); change it to use the project path-alias by importing WsManager
from "@/ws-manager" so it follows the repo convention and avoids future
path-drift (update the import statement that references WsManager accordingly).
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/transport-classifier.ts`:
- Around line 38-42: The endpoint check in transport-classifier.ts currently
uses pathname.endsWith("/responses") which is too permissive; update the logic
in the block that reads const pathname = session.requestUrl.pathname so it only
accepts the exact proxy route (e.g., compare pathname === "/v1/responses") or
validate path segments (e.g., split pathname by "/" and ensure segments match
["", "v1", "responses"] or use a strict RegExp like /^\/v1\/responses(?:\/)?$/)
and return the same { transport: "http", reason: "not_responses_endpoint" } for
anything that doesn't match.
In `@src/app/v1/_lib/ws/ingress-handler.ts`:
- Line 12: In src/app/v1/_lib/ws/ingress-handler.ts replace the relative import
of extractApiKeyFromHeaders from "../proxy/auth-guard" with the project
path-alias style using "@/..." (e.g. import extractApiKeyFromHeaders from
"@/app/v1/_lib/proxy/auth-guard") so internal imports consistently use the `@/`
alias; keep the imported symbol name extractApiKeyFromHeaders unchanged and
ensure your tsconfig/paths supports the alias.
In `@src/app/v1/_lib/ws/outbound-adapter.ts`:
- Line 5: Import in outbound-adapter.ts uses a relative path; change the import
of toWebSocketUrl to use the project alias by replacing
"../proxy/transport-classifier" with "@/app/v1/_lib/proxy/transport-classifier"
so the file imports toWebSocketUrl via the `@/` src alias consistent with
repository conventions.
In `@src/app/v1/_lib/ws/session-continuity.ts`:
- Around line 4-5: Replace the relative imports with the project path-alias `@/`
so the module uses aliases rather than relative paths: change the import
references for the types SettlementResult, TurnMeta, and WsAuthContext in
session-continuity.ts to use `@/`-prefixed paths that point into the src tree
(e.g., import SettlementResult from '@/...' and import { TurnMeta, WsAuthContext
} from '@/...'), ensuring the alias matches your tsconfig/webpack config and
that the exported type names (SettlementResult, TurnMeta, WsAuthContext) remain
unchanged.
In `@src/lib/ws/frame-parser.ts`:
- Around line 38-42: The error message construction in the frame-parser's
validation handling uses firstIssue.path.join(".") which yields an empty string
when path is [], producing messages like ": Required"; update the logic that
builds message (where result.error.issues[0] / firstIssue is used) to detect an
empty path (e.g., firstIssue.path.length === 0) and omit the leading colon/empty
prefix so the message becomes just firstIssue.message, otherwise keep
`${firstIssue.path.join(".")}: ${firstIssue.message}`.
In `@src/server/ws-manager.ts`:
- Around line 73-84: 当前 close() 返回的 Promise 可能会被某些挂起的客户端关闭操作阻塞；在 close()（使用
heartbeatInterval、this.wss 和 this.wss.close()）里为 this.wss.close()
的异步等待添加超时保护：将现有 new Promise 包装成带超时的 Promise.race，一方面保留
clearInterval(this.heartbeatInterval) 和遍历 this.wss.clients 调用 ws.close(1001,...
)，另一方面在超时（例如 5s）触发时调用 this.wss.clients.forEach(ws => ws.terminate()) 或直接调用
this.wss.close() 的强制完成分支并 resolve，以确保 close() 在可控时间内返回。
In `@tests/unit/proxy/transport-classifier.test.ts`:
- Around line 233-236: Add a unit test covering the trailing-slash edge case for
toWebSocketUrl: create an it block (similar to the existing "preserves existing
/v1/responses path") that calls
toWebSocketUrl("https://api.openai.com/v1/responses/") and asserts the returned
value preserves the trailing slash and uses wss (e.g.,
"wss://api.openai.com/v1/responses/"); reference the existing test name and the
toWebSocketUrl function so the new test sits alongside the current case and
verifies the "/v1/responses/" boundary behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0cdcc897-d445-40a8-8c2f-46aad480eb67
📒 Files selected for processing (59)
Dockerfiledeploy/Dockerfiledeploy/Dockerfile.devdrizzle/0079_quick_blink.sqldrizzle/meta/0079_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/provider-chain.jsonmessages/en/settings/config.jsonmessages/en/settings/providers/form/apiTest.jsonmessages/ja/provider-chain.jsonmessages/ja/settings/config.jsonmessages/ja/settings/providers/form/apiTest.jsonmessages/ru/provider-chain.jsonmessages/ru/settings/config.jsonmessages/ru/settings/providers/form/apiTest.jsonmessages/zh-CN/provider-chain.jsonmessages/zh-CN/settings/config.jsonmessages/zh-CN/settings/providers/form/apiTest.jsonmessages/zh-TW/provider-chain.jsonmessages/zh-TW/settings/config.jsonmessages/zh-TW/settings/providers/form/apiTest.jsonnext.config.tspackage.jsonsrc/actions/system-config.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/[locale]/settings/providers/_components/forms/ws-test-status.tsxsrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/transport-classifier.tssrc/app/v1/_lib/ws/billing-parity.tssrc/app/v1/_lib/ws/event-bridge.tssrc/app/v1/_lib/ws/ingress-handler.tssrc/app/v1/_lib/ws/outbound-adapter.tssrc/app/v1/_lib/ws/session-continuity.tssrc/drizzle/schema.tssrc/lib/config/system-settings-cache.tssrc/lib/provider-testing/ws-probe.tssrc/lib/provider-testing/ws-types.tssrc/lib/validation/schemas.tssrc/lib/ws/frame-parser.tssrc/lib/ws/frames.tssrc/repository/_shared/transformers.tssrc/repository/system-config.tssrc/server/index.tssrc/server/ws-manager.tssrc/types/message.tssrc/types/system-config.tstests/unit/lib/config/system-settings-responses-websocket-toggle.test.tstests/unit/lib/ws/frame-parser.test.tstests/unit/lib/ws/frames.test.tstests/unit/provider-testing/ws-probe.test.tstests/unit/provider-testing/ws-test-status.test.tsxtests/unit/proxy/transport-classifier.test.tstests/unit/server/ws-manager.test.tstests/unit/ws/billing-parity.test.tstests/unit/ws/event-bridge.test.tstests/unit/ws/ingress-handler.test.tstests/unit/ws/outbound-adapter.test.tstests/unit/ws/session-continuity.test.ts
Comment on lines
+61
to
63
| # TODO: Switch to custom server entry for WebSocket support once ingress handler is ready | ||
| # CMD ["node", "src/server/index.js"] | ||
| CMD ["node", "server.js"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查 server.js 和 src/server/index.js 的存在及内容
# 查找服务器相关文件
echo "=== 查找服务器入口文件 ==="
fd -t f -e js -e ts "^(server|index)\.(js|ts)$" --max-depth 2
echo ""
echo "=== 检查 package.json 中的启动脚本 ==="
cat package.json | jq '.scripts | to_entries | map(select(.key | test("start")))' 2>/dev/null || grep -A 5 '"scripts"' package.jsonRepository: ding113/claude-code-hub
Length of output: 259
🏁 Script executed:
#!/bin/bash
# 检查 Next.js 构建配置和服务器文件结构
echo "=== 查找 Next.js 配置文件 ==="
fd -t f "next\.config\.(js|ts|mjs)" .
echo ""
echo "=== 检查 src/server 目录结构 ==="
fd -t f . src/server/ 2>/dev/null || echo "src/server/ 目录不存在或为空"
echo ""
echo "=== 检查 .next 构建输出目录 ==="
ls -la .next/ 2>/dev/null | head -20 || echo ".next/ 目录不存在"
echo ""
echo "=== 查找所有 server 相关文件 ==="
fd -t f "server" . --exclude "node_modules" --exclude ".git" --exclude ".next"
echo ""
echo "=== 检查 Dockerfile 的完整内容 ==="
cat deploy/DockerfileRepository: ding113/claude-code-hub
Length of output: 2454
TODO 注释已过时，Dockerfile 应更新以使用自定义服务器入口。
WebSocket 入站处理器已在本 PR 中实现（src/server/index.ts 和 ws-manager.ts 存在），但 Dockerfile 仍在使用默认的 Next.js 服务器（server.js 来自 .next/standalone）。请将 CMD 更新为使用自定义服务器入口点 src/server/index.ts，或澄清为何目前仍使用默认服务器。
同时，注释中对 src/server/index.js 的引用不正确，实际文件为 src/server/index.ts。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy/Dockerfile` around lines 61 - 63, Update the Dockerfile CMD to run the
custom server entry used for WebSocket handling instead of the default Next.js
standalone server: replace the existing CMD that launches server.js with one
that starts your custom server entry (src/server/index.ts compiled/started
appropriately) and remove or correct the outdated TODO/comment that references
src/server/index.js; ensure the Docker runtime/start command aligns with how you
build and run TypeScript (e.g., use the compiled JS entry or a node wrapper) so
the custom entry that references ws-manager.ts is actually launched.
Comment on lines
+28
to
30
| # TODO: Switch to custom server entry for WebSocket support once ingress handler is ready | ||
| # CMD ["node", "src/server/index.js"] | ||
| CMD ["node", "server.js"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Dockerfile CMD =="
sed -n '28,30p' Dockerfile
echo
echo "== package.json scripts =="
node <<'NODE'
const pkg = require("./package.json");
console.log("start =", pkg.scripts?.start);
console.log("start:ws =", pkg.scripts?.["start:ws"]);
NODE
echo
echo "== custom server entry =="
fd index.ts src/server -x sed -n '1,220p' {}
echo
echo "== runtime/build references =="
rg -n --glob '!node_modules' 'src/server/index|server\.js|/v1/responses|WebSocketServer|ws-manager' .Repository: ding113/claude-code-hub
Length of output: 18518
Docker 当前运行的是 Next.js 独立输出，会跳过自定义 WS 服务器。
容器启动 server.js（Next.js standalone from .next/standalone/），而完整的 HTTP+WS 引导逻辑在 src/server/index.ts 中。该自定义服务器已正确实现：
- 在同一端口附加 WebSocket 管理器
/v1/responses路径配置完成- WsManager 和 WebSocketServer 依赖已导入
问题：Docker 目前运行的 standalone 输出不包含自定义服务器代码，导致容器内 WebSocket 无法工作。
修复：修改 Dockerfile 第 28-30 行，将 CMD ["node", "server.js"] 改为：
CMD ["node", "--import", "tsx", "src/server/index.ts"]
或更新构建流程，确保自定义服务器逻辑被编译进 server.js。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Dockerfile` around lines 28 - 30, The Dockerfile currently launches the
Next.js standalone server (CMD running server.js) which skips the custom HTTP+WS
bootstrap in src/server/index.ts; update the Dockerfile to run the custom server
entry instead (start node with tsx importing src/server/index.ts) or
alternatively change the build so that the compiled server.js includes the
custom server code from src/server/index.ts; ensure the container runs the entry
that initializes WsManager/WebSocketServer and mounts the /v1/responses route
rather than the standalone Next output.
| "dev": "next dev --port 13500", | ||
| "build": "next build && (node scripts/copy-version-to-standalone.cjs || bun scripts/copy-version-to-standalone.cjs)", | ||
| "start": "next start", | ||
| "start:ws": "node --import tsx src/server/index.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node <<'NODE'
const pkg = require("./package.json");
console.log("start:ws =", pkg.scripts?.["start:ws"]);
console.log("tsx =", pkg.dependencies?.tsx ?? pkg.devDependencies?.tsx ?? "MISSING");
NODERepository: ding113/claude-code-hub
Length of output: 129
🏁 Script executed:
# Check for tsx references in package-lock.json or yarn.lock
if [ -f package-lock.json ]; then
echo "=== Checking package-lock.json for tsx ==="
grep -c "\"tsx\"" package-lock.json || echo "tsx not in package-lock.json"
fi
# Check yarn.lock
if [ -f yarn.lock ]; then
echo "=== Checking yarn.lock for tsx ==="
grep -c "tsx" yarn.lock || echo "tsx not in yarn.lock"
fi
# Check if src/server/index.ts exists
echo "=== Checking if src/server/index.ts exists ==="
if [ -f src/server/index.ts ]; then
echo "File exists"
head -5 src/server/index.ts
else
echo "File does NOT exist"
fi
# Check full package.json for any tsx references
echo "=== Full dependencies and devDependencies ==="
node <<'NODE'
const pkg = require("./package.json");
console.log("All dependencies:", Object.keys(pkg.dependencies || {}));
console.log("All devDependencies:", Object.keys(pkg.devDependencies || {}));
NODERepository: ding113/claude-code-hub
Length of output: 2159
🏁 Script executed:
# Check for tsconfig.json
echo "=== Checking for tsconfig.json ==="
if [ -f tsconfig.json ]; then
echo "tsconfig.json exists"
head -10 tsconfig.json
fi
# Check if tsx is used in other scripts
echo "=== Checking all npm scripts for tsx usage ==="
node <<'NODE'
const pkg = require("./package.json");
const scripts = pkg.scripts || {};
for (const [name, cmd] of Object.entries(scripts)) {
if (cmd.includes('tsx')) {
console.log(`${name}: ${cmd}`);
}
}
NODE
# Check if there's a compiled TypeScript somewhere
echo "=== Checking for dist or build directories ==="
ls -la | grep -E "^d" | grep -E "(dist|build|out)" || echo "No dist/build directories found"Repository: ding113/claude-code-hub
Length of output: 531
start:ws 缺少 tsx 依赖声明。
脚本使用 node --import tsx 但 tsx 未在 package.json 中声明，干净安装后会在启动时失败。
建议方案：
- 添加
tsx到 devDependencies，或 - 改用
tsx src/server/index.ts命令
修正示例
- "start:ws": "node --import tsx src/server/index.ts",
+ "start:ws": "tsx src/server/index.ts",
@@
"devDependencies": {
+ "tsx": "^4.20.3",
"@biomejs/biome": "^2",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 9, package.json 中的 "start:ws" 脚本引用了 node --import tsx
但项目未声明 tsx 依赖，导致在干净安装后启动失败；修复方法是要么在 package.json 的 devDependencies 中添加 "tsx"（并运行
npm/yarn 安装），要么修改 "start:ws" 脚本为直接使用 tsx 可执行命令（例如 "tsx src/server/index.ts"），请更新
package.json 中的 "start:ws" 脚本或 devDependencies 以确保 tsx 在运行时可用。
Comment on lines
+53
to
+56
| {result.wsFallbackReason && ( | ||
| <div data-testid="ws-fallback-reason" className="mt-2 text-xs"> | ||
| <span className="text-muted-foreground">{t("ws.fallbackReason")}:</span>{" "} | ||
| <span className="text-destructive">{result.wsFallbackReason}</span> |
There was a problem hiding this comment.
不要直接向用户渲染 wsFallbackReason 原始值。
这里会把分类器/探测层返回的 reason code 直接显示到界面，跨语言场景下会暴露未翻译的内部标识，并且把 UI 文案和后端枚举值绑死。请改成稳定的 reason code → t() 映射，未知值再回退到通用提示。
As per coding guidelines, "All user-facing strings must use i18n (5 languages supported: zh-CN, zh-TW, en, ja, ru). Never hardcode display text".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/settings/providers/_components/forms/ws-test-status.tsx
around lines 53 - 56, Do not render result.wsFallbackReason directly; instead
map that internal reason code to an i18n key and pass the key to t() with a
fallback. Add a small mapping object (e.g., wsFallbackReasonKeyMap) that maps
known reason codes to translation keys like "ws.fallbackReason.network",
"ws.fallbackReason.auth", etc., then compute const reasonKey =
wsFallbackReasonKeyMap[result.wsFallbackReason] ?? "ws.fallbackReason.unknown"
and render t(reasonKey) where result.wsFallbackReason is currently shown;
optionally keep the raw value in a debug log but never expose it in the UI.
Ensure translation keys exist for all five supported locales.
Comment on lines
+145
to
+164
| export function wsUsageToMetrics(usage?: ResponseUsage): UsageMetrics | null { | ||
| if (!usage) return null; | ||
|
|
||
| // Access passthrough fields via Record cast | ||
| const raw = usage as Record<string, unknown>; | ||
|
|
||
| return { | ||
| input_tokens: usage.input_tokens, | ||
| output_tokens: usage.output_tokens, | ||
| cache_creation_input_tokens: | ||
| typeof raw.cache_creation_input_tokens === "number" | ||
| ? raw.cache_creation_input_tokens | ||
| : undefined, | ||
| cache_read_input_tokens: | ||
| typeof raw.cache_read_input_tokens === "number" ? raw.cache_read_input_tokens : undefined, | ||
| // WS terminal payloads do not include 5m/1h split or cache_ttl; | ||
| // leaving these undefined causes downstream pricing to fall back | ||
| // to the unified cache_creation_input_tokens path. | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, understand the file structure and related code
fd "billing-parity\|response-handler\|key\.ts" --type fRepository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Search for input_tokens_details usage in the codebase
rg "input_tokens_details" --type ts --type tsx -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Search for cached_tokens references
rg "cached_tokens" --type ts --type tsx -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Check the ResponseUsage type definition
rg "type ResponseUsage\|interface ResponseUsage" --type ts --type tsx -A 20Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Look for how billing is calculated and if there's a difference between WS and HTTP paths
rg "calculateRequestCost\|wsUsageToMetrics" --type ts --type tsx -B 3 -A 5Repository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# List files in the repository to understand structure
fd . --type f --name "*.ts" | grep -E "(billing|response)" | head -20Repository: ding113/claude-code-hub
Length of output: 317
🏁 Script executed:
# Search for input_tokens_details and cached_tokens with correct syntax
rg "input_tokens_details" --type ts -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 1965
🏁 Script executed:
# Search for ResponseUsage type definition
rg "ResponseUsage" --type ts -B 2 -A 10Repository: ding113/claude-code-hub
Length of output: 11025
🏁 Script executed:
# Read the billing-parity.ts file completely
cat src/app/v1/_lib/ws/billing-parity.tsRepository: ding113/claude-code-hub
Length of output: 10672
🏁 Script executed:
# Check for response-handler and how it handles cache tokens
fd "response-handler" --type fRepository: ding113/claude-code-hub
Length of output: 361
🏁 Script executed:
# Check the ResponseUsage schema definition in frames.ts
rg "UsageSchema\|ResponseUsage" src/lib/ws/frames.ts -A 30Repository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Check if input_tokens_details is mentioned in the WS frames or documentation
rg "input_tokens_details" src/lib/ws/ --type tsRepository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Look at the HTTP response-handler to see how it handles input_tokens_details
rg -A 10 "input_tokens_details" src/app/v1/_lib/proxy/response-handler.tsRepository: ding113/claude-code-hub
Length of output: 685
🏁 Script executed:
# Check test cases for wsUsageToMetrics to see if nested cached_tokens is tested
cat tests/unit/ws/billing-parity.test.ts | grep -A 20 "wsUsageToMetrics"Repository: ding113/claude-code-hub
Length of output: 2055
wsUsageToMetrics 缺少对嵌套 input_tokens_details.cached_tokens 的处理，导致 WS 账单与 HTTP 路径失配
HTTP 路径在 response-handler.ts 中已实现从嵌套 input_tokens_details.cached_tokens 提取缓存读 token 的逻辑（当顶层 cache_read_input_tokens 不存在时），但 WS 路径的 wsUsageToMetrics 函数仅读取顶层 passthrough 字段，未实现这个回退逻辑。当 OpenAI 仅在嵌套结构中返回缓存 token 时，WS 终端事件会漏掉该数据，导致 usage 和 cost 计算与 HTTP 路径发生偏差。
建议修改 wsUsageToMetrics 添加对 input_tokens_details.cached_tokens 的检查（作为 cache_read_input_tokens 的回退），并补充对应的回归测试用例。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/v1/_lib/ws/billing-parity.ts` around lines 145 - 164, The
wsUsageToMetrics function currently only reads top-level passthrough fields and
misses the nested cached tokens fallback; update wsUsageToMetrics (and its usage
of ResponseUsage/raw) to check for input_tokens_details?.cached_tokens and use
that value as cache_read_input_tokens when raw.cache_read_input_tokens is
undefined or not a number, mirroring the logic in response-handler.ts; add a
unit test that supplies a ResponseUsage with no top-level
cache_read_input_tokens but with input_tokens_details.cached_tokens to ensure
the converted UsageMetrics includes cache_read_input_tokens.
Comment on lines
+41
to
+64
| export const ResponseCreateFrameSchema = z.object({ | ||
| type: z.literal("response.create"), | ||
| response: z | ||
| .object({ | ||
| model: z.string().min(1), | ||
| input: z.array(InputItemSchema).optional(), | ||
| instructions: z.string().optional(), | ||
| max_output_tokens: z.number().int().positive().optional(), | ||
| metadata: z.record(z.string(), z.string()).optional(), | ||
| parallel_tool_calls: z.boolean().optional(), | ||
| previous_response_id: z.string().optional(), | ||
| reasoning: ReasoningConfigSchema.optional(), | ||
| store: z.boolean().optional(), | ||
| temperature: z.number().optional(), | ||
| tool_choice: z.union([z.string(), z.object({}).passthrough()]).optional(), | ||
| tools: z.array(z.any()).optional(), | ||
| top_p: z.number().optional(), | ||
| truncation: z.enum(["auto", "disabled"]).optional(), | ||
| user: z.string().optional(), | ||
| service_tier: ServiceTierSchema.optional(), | ||
| stream: z.boolean().optional(), | ||
| prompt_cache_key: z.string().optional(), | ||
| }) | ||
| .passthrough(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/ws/frames.ts | head -100Repository: ding113/claude-code-hub
Length of output: 4334
🏁 Script executed:
# Search for tests related to ResponseCreateFrameSchema
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | head -20Repository: ding113/claude-code-hub
Length of output: 1109
🏁 Script executed:
# Look for usages of ResponseCreateFrameSchema in the codebase
rg "ResponseCreateFrameSchema" -A 3 -B 3Repository: ding113/claude-code-hub
Length of output: 6276
🏁 Script executed:
# Check if there are any other related schemas that might define input handling
rg "InputItemSchema" -A 2 -B 2Repository: ding113/claude-code-hub
Length of output: 623
🏁 Script executed:
# Search for ResponseRequest type definition
rg "type ResponseRequest" -A 20Repository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Look at the test file to see what input patterns are being tested
cat -n tests/unit/lib/ws/frames.test.ts | grep -A 50 "ResponseCreateFrameSchema"Repository: ding113/claude-code-hub
Length of output: 7122
🏁 Script executed:
# Search for input field usage in tests
rg "input:" tests/unit/lib/ws/frames.test.ts -B 3 -A 3Repository: ding113/claude-code-hub
Length of output: 325
🌐 Web query:
OpenAI Responses API input field documentation string array
💡 Result:
In the Responses API (POST /v1/responses), the request body field input is documented as:
input: optionalstringorarrayofResponseInputItem— “Text, image, or file inputs to the model, used to generate a response.” [1]
If you pass an array, it’s not an array of plain strings; it’s an array of typed input items (e.g., messages, or content parts like input_text, input_image, input_file). For text parts specifically, the docs define:
TextInput = string(single text input), orResponseInputMessageContentList = array of ResponseInputContent, where a text part is{"type":"input_text","text":"..."}. [2]
Minimal examples
1) Single string input
{ "model": "gpt-4.1", "input": "Hello!" }[1]
2) Array input with explicit text items
{
"model": "gpt-4.1",
"input": [
{
"role": "user",
"content": [
{ "type": "input_text", "text": "Hello!" },
{ "type": "input_text", "text": "Second chunk of text." }
]
}
]
}[2]
Sources: OpenAI API Reference — Responses: Create a response (input type + content item schema). [1] [2]
🏁 Script executed:
# Search more broadly for ResponseRequest
find . -type f \( -name "*.ts" -o -name "*.tsx" \) -path "*/codex/types/*" | head -20Repository: ding113/claude-code-hub
Length of output: 152
🏁 Script executed:
# Search for where ResponseRequest is defined
rg "type ResponseRequest\s*=" -A 15Repository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Check if there are any usages of input as string anywhere in the codebase
rg "input\s*:\s*['\"]" --type ts --type tsxRepository: ding113/claude-code-hub
Length of output: 94
🏁 Script executed:
# Check the response.ts file for ResponseRequest definition
cat -n src/app/v1/_lib/codex/types/response.ts | head -100Repository: ding113/claude-code-hub
Length of output: 2847
🏁 Script executed:
# Also search in compatible.ts
cat -n src/app/v1/_lib/codex/types/compatible.ts | head -100Repository: ding113/claude-code-hub
Length of output: 2833
🏁 Script executed:
# Search for input type in response-related files
rg "input" src/app/v1/_lib/codex/types/ -A 2 -B 2Repository: ding113/claude-code-hub
Length of output: 1312
🏁 Script executed:
# Search for any conversion or parsing logic for input field
rg "parseClientFrame\|parseInput\|input.*string" src/ -A 3 -B 3Repository: ding113/claude-code-hub
Length of output: 49
🏁 Script executed:
# Check if there are any comments about input handling
rg "input" src/lib/ws/frames.ts -B 2 -A 2Repository: ding113/claude-code-hub
Length of output: 711
response.input 应支持字符串和数组两种格式
OpenAI Responses API 的 input 字段支持单个字符串或数组两种格式，但当前模式仅接受数组。这导致 input: "Hello!" 这类合法请求被拒绝，造成 HTTP/WS 语义不一致。
建议修改为：z.union([z.string(), z.array(InputItemSchema)]).optional()，并补充测试用例验证字符串输入。
建议修改
export const ResponseCreateFrameSchema = z.object({
type: z.literal("response.create"),
response: z
.object({
model: z.string().min(1),
- input: z.array(InputItemSchema).optional(),
+ input: z.union([z.string(), z.array(InputItemSchema)]).optional(),
instructions: z.string().optional(),
max_output_tokens: z.number().int().positive().optional(),📝 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.
Suggested change
| export const ResponseCreateFrameSchema = z.object({ | |
| type: z.literal("response.create"), | |
| response: z | |
| .object({ | |
| model: z.string().min(1), | |
| input: z.array(InputItemSchema).optional(), | |
| instructions: z.string().optional(), | |
| max_output_tokens: z.number().int().positive().optional(), | |
| metadata: z.record(z.string(), z.string()).optional(), | |
| parallel_tool_calls: z.boolean().optional(), | |
| previous_response_id: z.string().optional(), | |
| reasoning: ReasoningConfigSchema.optional(), | |
| store: z.boolean().optional(), | |
| temperature: z.number().optional(), | |
| tool_choice: z.union([z.string(), z.object({}).passthrough()]).optional(), | |
| tools: z.array(z.any()).optional(), | |
| top_p: z.number().optional(), | |
| truncation: z.enum(["auto", "disabled"]).optional(), | |
| user: z.string().optional(), | |
| service_tier: ServiceTierSchema.optional(), | |
| stream: z.boolean().optional(), | |
| prompt_cache_key: z.string().optional(), | |
| }) | |
| .passthrough(), | |
| export const ResponseCreateFrameSchema = z.object({ | |
| type: z.literal("response.create"), | |
| response: z | |
| .object({ | |
| model: z.string().min(1), | |
| input: z.union([z.string(), z.array(InputItemSchema)]).optional(), | |
| instructions: z.string().optional(), | |
| max_output_tokens: z.number().int().positive().optional(), | |
| metadata: z.record(z.string(), z.string()).optional(), | |
| parallel_tool_calls: z.boolean().optional(), | |
| previous_response_id: z.string().optional(), | |
| reasoning: ReasoningConfigSchema.optional(), | |
| store: z.boolean().optional(), | |
| temperature: z.number().optional(), | |
| tool_choice: z.union([z.string(), z.object({}).passthrough()]).optional(), | |
| tools: z.array(z.any()).optional(), | |
| top_p: z.number().optional(), | |
| truncation: z.enum(["auto", "disabled"]).optional(), | |
| user: z.string().optional(), | |
| service_tier: ServiceTierSchema.optional(), | |
| stream: z.boolean().optional(), | |
| prompt_cache_key: z.string().optional(), | |
| }) | |
| .passthrough(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/ws/frames.ts` around lines 41 - 64, The ResponseCreateFrameSchema
currently enforces response.input as an array only; update the schema
(ResponseCreateFrameSchema) so response.input accepts either a string or an
array by replacing the input type with z.union([z.string(),
z.array(InputItemSchema)]).optional() (keep .passthrough() on the parent object)
and add unit tests for the parser/validator to cover both input: "Hello!" and
input: ["..."] cases to ensure HTTP/WS semantics accept single-string input as
well as arrays; reference InputItemSchema when implementing the union so array
validation remains unchanged.
Comment on lines
+341
to
+344
| // Responses WebSocket 配置字段（如果提供） | ||
| if (payload.enableResponsesWebSocket !== undefined) { | ||
| updates.enableResponsesWebSocket = payload.enableResponsesWebSocket; | ||
| } |
There was a problem hiding this comment.
把 enableResponsesWebSocket 一并放进 returning(...) 投影。
这里已经把 payload.enableResponsesWebSocket 写进 updates，但更新后的返回行没有选出这个列，toSystemSettings(updated) 只能看到 undefined 并回退默认值。结果是写库成功后，调用方马上拿到的仍可能是 false，保存开关时 UI/缓存状态会立刻反弹。
建议修改
verboseProviderError: systemSettings.verboseProviderError,
enableHttp2: systemSettings.enableHttp2,
+ enableResponsesWebSocket: systemSettings.enableResponsesWebSocket,
interceptAnthropicWarmupRequests: systemSettings.interceptAnthropicWarmupRequests,Also applies to: 409-442
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/repository/system-config.ts` around lines 341 - 344, The update routine
sets payload.enableResponsesWebSocket into updates but doesn’t include that
column in the returning(...) projection, so toSystemSettings(updated) receives
undefined; modify the DB update call to include enableResponsesWebSocket in the
returning(...) list (the same projection used for other fields) so the updated
row returned to toSystemSettings contains the real value for
enableResponsesWebSocket, and verify the toSystemSettings/updates mapping uses
the same property name.
Comment on lines
+7
to
+8
| const hostname = process.env.HOSTNAME || "0.0.0.0"; | ||
| const port = parseInt(process.env.PORT || "3000", 10); |
There was a problem hiding this comment.
不要用 process.env.HOSTNAME 作为监听地址。
在容器和 Kubernetes 环境里，HOSTNAME 通常是 pod/container 名称，不是可绑定的地址；这里会让服务优先绑定到一个随机主机名，启动时很容易直接失败。建议改成专门的 HOST 变量，默认仍然回落到 0.0.0.0。
建议修改
-const hostname = process.env.HOSTNAME || "0.0.0.0";
+const hostname = process.env.HOST || "0.0.0.0";📝 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.
Suggested change
| const hostname = process.env.HOSTNAME || "0.0.0.0"; | |
| const port = parseInt(process.env.PORT || "3000", 10); | |
| const hostname = process.env.HOST || "0.0.0.0"; | |
| const port = parseInt(process.env.PORT || "3000", 10); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/index.ts` around lines 7 - 8, The code uses process.env.HOSTNAME
for the bind address (const hostname), which is unsafe in containers; change it
to read from process.env.HOST with a fallback to "0.0.0.0" (i.e., replace
references to process.env.HOSTNAME in the hostname initialization with
process.env.HOST || "0.0.0.0") so the server binds to a proper network interface
instead of a pod/container name; keep the existing parseInt for port unchanged.
Comment on lines
+24
to
+39
| // Placeholder connection handler (will be replaced by ingress-handler in T6) | ||
| wsManager.onConnection((ws, req) => { | ||
| const url = new URL(req.url || "/", `http://${req.headers.host || "localhost"}`); | ||
| console.log(`[WS] New connection on ${url.pathname}`); | ||
|
|
||
| ws.on("message", () => { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: { | ||
| type: "server_error", | ||
| message: "WebSocket ingress not yet initialized", | ||
| }, | ||
| }) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
这里还是占位 WS 处理，/v1/responses 实际不会工作。
当前连接建立后没有接入真正的 ingress / bridge 流程，任意消息都会被回成 server_error: "WebSocket ingress not yet initialized"。如果这个文件就是生产入口，那么开关打开后 WebSocket 模式会在建连后立刻失败。请在这里接线真实的 WsIngressHandler/pipeline，而不是保留占位实现。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/index.ts` around lines 24 - 39, The placeholder WebSocket
connection handler (wsManager.onConnection) immediately replies with a
server_error and never wires the real ingress/bridge; replace the stubbed
message handler with the actual WsIngressHandler/pipeline so connections to
/v1/responses are forwarded to the real ingress logic. Locate the
wsManager.onConnection callback, instantiate or retrieve the WsIngressHandler
(or existing pipeline) and attach it to the ws connection and req (handing over
message/event handling and close/error propagation), removing the current
hardcoded ws.on("message", ...) that returns the "WebSocket ingress not yet
initialized" error.
Comment on lines
+43
to
+47
| const shutdown = async () => { | ||
| console.log("[Server] Shutting down..."); | ||
| await wsManager.close(); | ||
| server.close(); | ||
| process.exit(0); |
There was a problem hiding this comment.
等待 HTTP server 真正关闭后再退出进程。
server.close() 是异步的，这里马上 process.exit(0) 会截断仍在处理中的 HTTP 请求，和“graceful shutdown”的目标相反。至少要把 server.close 包成 Promise 并等待完成。
建议修改
const shutdown = async () => {
console.log("[Server] Shutting down...");
await wsManager.close();
- server.close();
+ await new Promise<void>((resolve, reject) => {
+ server.close((error) => {
+ if (error) reject(error);
+ else resolve();
+ });
+ });
process.exit(0);
};📝 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.
Suggested change
| const shutdown = async () => { | |
| console.log("[Server] Shutting down..."); | |
| await wsManager.close(); | |
| server.close(); | |
| process.exit(0); | |
| const shutdown = async () => { | |
| console.log("[Server] Shutting down..."); | |
| await wsManager.close(); | |
| await new Promise<void>((resolve, reject) => { | |
| server.close((error) => { | |
| if (error) reject(error); | |
| else resolve(); | |
| }); | |
| }); | |
| process.exit(0); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/index.ts` around lines 43 - 47, The shutdown function currently
calls server.close() and immediately process.exit(0), which can terminate
in-flight HTTP requests; change shutdown to await server.close by wrapping
server.close in a Promise (e.g., new Promise((resolve, reject) =>
server.close(err => err ? reject(err) : resolve()))) before calling
process.exit(0), keep awaiting wsManager.close() as is, and handle errors from
the awaited server.close to log and exit with an appropriate code instead of
exiting immediately.
| "build": "next build && (node scripts/copy-version-to-standalone.cjs || bun scripts/copy-version-to-standalone.cjs)", | ||
| "start": "next start", | ||
| "start:ws": "node --import tsx src/server/index.ts", | ||
| "lint": "biome check .", |
Contributor
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] start:ws will fail in clean installs (missing tsx)
Evidence (package.json:9):
"start:ws": "node --import tsx src/server/index.ts"Why this is a problem: node --import tsx ... requires the tsx loader to be installed in node_modules. tsx is not listed in dependencies/devDependencies, so bun run start:ws breaks in CI/containers and for any fresh checkout.
Suggested fix (prefer Bun since this repo already uses Bun):
{
"scripts": {
"start:ws": "bun src/server/index.ts"
}
}
|
|
||
| // Placeholder connection handler (will be replaced by ingress-handler in T6) | ||
| wsManager.onConnection((ws, req) => { | ||
| const url = new URL(req.url || "/", `http://${req.headers.host || "localhost"}`); |
Contributor
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] WS connections are hard-wired to a placeholder handler
Evidence (src/server/index.ts:25):
wsManager.onConnection((ws, req) => {
// ...
message: "WebSocket ingress not yet initialized",
});Why this is a problem: Starting the custom server will accept /v1/responses upgrades, but every client message is answered with a hardcoded error frame. This makes the WS endpoint unusable even when the system toggle is enabled.
Suggested fix: Replace the placeholder with the real ingress registration (and only expose start:ws / Docker entrypoint once WsIngressHandler.handleTurn() is implemented).
import { registerIngressHandler } from "../app/v1/_lib/ws/ingress-handler";
// ...
const wsManager = new WsManager(server);
registerIngressHandler(wsManager);
| const apiKey = extractApiKeyFromHeaders({ | ||
| authorization: this.req.headers.authorization ?? null, | ||
| "x-api-key": (this.req.headers["x-api-key"] as string) ?? null, | ||
| "x-goog-api-key": (this.req.headers["x-goog-api-key"] as string) ?? null, |
Contributor
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Unsafe header casts can crash auth parsing on duplicate headers
Evidence (src/app/v1/_lib/ws/ingress-handler.ts:101):
"x-api-key": (this.req.headers["x-api-key"] as string) ?? null,Why this is a problem: In Node, IncomingMessage.headers[...] values can be string | string[] | undefined. The as string cast does not coerce at runtime; if a client sends duplicate x-api-key (or x-goog-api-key) headers, this becomes an array and extractApiKeyFromHeaders() will throw when it calls .trim(). That turns a malformed client request into a server-side exception.
Suggested fix: Normalize header values before passing them into extractApiKeyFromHeaders().
const xApiKey = this.req.headers["x-api-key"];
const xGoogApiKey = this.req.headers["x-goog-api-key"];
const authorization = this.req.headers.authorization;
const apiKey = extractApiKeyFromHeaders({
authorization: typeof authorization === "string" ? authorization : null,
"x-api-key": typeof xApiKey === "string" ? xApiKey : null,
"x-goog-api-key": typeof xGoogApiKey === "string" ? xGoogApiKey : null,
});
| options?: IngressHandlerOptions | ||
| ): void { | ||
| wsManager.onConnection(async (ws, req) => { | ||
| const handler = new WsIngressHandler(ws, req, options); |
Contributor
There was a problem hiding this comment.
[HIGH] [ERROR-SILENT] Unhandled rejection risk in registerIngressHandler connection callback
Evidence (src/app/v1/_lib/ws/ingress-handler.ts:356):
wsManager.onConnection(async (ws, req) => {
const handler = new WsIngressHandler(ws, req, options);
const ok = await handler.start();
// ...
});Why this is a problem: WsManager.onConnection() does not await the handler return value. If handler.start() throws (DB outage, settings cache error, unexpected repo error), this becomes an unhandled promise rejection and can crash the process or drop the connection without a controlled close.
Suggested fix: Make the callback non-async and explicitly .catch() initialization failures with logging and a 1011 close.
wsManager.onConnection((ws, req) => {
const handler = new WsIngressHandler(ws, req, options);
handler.start().catch((err) => {
logger.error("[WsIngress] Connection init failed", { error: err });
ws.close(1011, "Internal server error");
});
});
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR is extremely large (10,589 LOC delta across 59 files), which makes regression risk and reviewability high. The WS server/ingress path currently contains multiple runtime blockers (placeholder connection handler, unsafe header assumptions, and unhandled async init), so enabling or relying on /v1/responses WebSocket transport is likely to fail in real deployments.
PR Size: XL
- Lines changed: 10589
- Files changed: 59
Split suggestions (recommended for XL):
- PR 1: DB/schema + cache + settings UI + i18n keys for
enableResponsesWebSocket - PR 2: WS frame schemas/parser + unit tests
- PR 3: Provider WS probe + UI surface (
WsTestStatus) + tests - PR 4: Custom server entry +
WsManager+ ingress wiring + production entrypoint changes - PR 5: (If intended) turn bridging/outbound adapter/event bridge/billing parity/session continuity integration
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 3 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
package.json:9(confidence 90):start:wsusesnode --import tsx ...buttsxis not declared, so the command fails in clean installs.src/server/index.ts:25(confidence 88): WS connections are wired to a placeholder handler that always returns an error frame; additionally, even after wiring ingress,WsIngressHandler.handleTurn()is still a placeholder.src/app/v1/_lib/ws/ingress-handler.ts:101(confidence 86):IncomingMessage.headers[...]can bestring[]; current casts can throw when.trim()is called.src/app/v1/_lib/ws/ingress-handler.ts:356(confidence 85):wsManager.onConnection(async ...)can create unhandled promise rejections if init throws.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
Applied the size/XL label to PR #887 and submitted the review.
Inline review comments posted (4 total), focused on diff lines with high-confidence runtime blockers:
package.json:9(start:wsdepends on undeclaredtsx)src/server/index.ts:25(WS path is still a placeholder handler)src/app/v1/_lib/ws/ingress-handler.ts:101(unsafe header casts can throw onstring[]headers)src/app/v1/_lib/ws/ingress-handler.ts:356(async connection callback can create unhandled rejections)
A full summary review comment was also posted on the PR (includes XL split suggestions and the issue table).
Connect all WebSocket components into a working end-to-end pipeline: - Add ProxySession.fromWebSocket() factory for synthetic sessions - Implement handleTurn() with guard pipeline, outbound adapter, event relay, billing settlement, and session continuity - Wire response.cancel to terminate upstream connections - Add integration tests for full orchestration flow
Comment on lines
+66
to
+74
| export function toWebSocketUrl(providerBaseUrl: string): string { | ||
| const url = new URL(providerBaseUrl); | ||
| url.protocol = "wss:"; | ||
| // Ensure path ends with /v1/responses | ||
| if (!url.pathname.endsWith("/v1/responses")) { | ||
| url.pathname = url.pathname.replace(/\/$/, "") + "/v1/responses"; | ||
| } | ||
| return url.toString(); | ||
| } |
There was a problem hiding this comment.
Path double-append bug when base URL includes /v1
toWebSocketUrl appends /v1/responses to the URL's existing pathname without clearing it first. When a provider is configured with the common base URL pattern https://api.openai.com/v1, the resulting WebSocket URL becomes wss://api.openai.com/v1/v1/responses instead of the correct wss://api.openai.com/v1/responses.
Example:
Input: https://api.openai.com/v1
Output: wss://api.openai.com/v1/v1/responses ← wrong
Wanted: wss://api.openai.com/v1/responses
The fix is to reset the pathname to the target path:
export function toWebSocketUrl(providerBaseUrl: string): string {
const url = new URL(providerBaseUrl);
url.protocol = "wss:";
url.pathname = "/v1/responses"; // Always set to target, don't append
return url.toString();
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/transport-classifier.ts
Line: 66-74
Comment:
**Path double-append bug when base URL includes `/v1`**
`toWebSocketUrl` appends `/v1/responses` to the URL's existing pathname without clearing it first. When a provider is configured with the common base URL pattern `https://api.openai.com/v1`, the resulting WebSocket URL becomes `wss://api.openai.com/v1/v1/responses` instead of the correct `wss://api.openai.com/v1/responses`.
Example:
```
Input: https://api.openai.com/v1
Output: wss://api.openai.com/v1/v1/responses ← wrong
Wanted: wss://api.openai.com/v1/responses
```
The fix is to reset the pathname to the target path:
```ts
export function toWebSocketUrl(providerBaseUrl: string): string {
const url = new URL(providerBaseUrl);
url.protocol = "wss:";
url.pathname = "/v1/responses"; // Always set to target, don't append
return url.toString();
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+24
to
+40
| // Placeholder connection handler (will be replaced by ingress-handler in T6) | ||
| wsManager.onConnection((ws, req) => { | ||
| const url = new URL(req.url || "/", `http://${req.headers.host || "localhost"}`); | ||
| console.log(`[WS] New connection on ${url.pathname}`); | ||
|
|
||
| ws.on("message", () => { | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: { | ||
| type: "server_error", | ||
| message: "WebSocket ingress not yet initialized", | ||
| }, | ||
| }) | ||
| ); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Placeholder handler is never replaced with registerIngressHandler
src/server/index.ts registers a placeholder onConnection handler (lines 25–40) but never calls registerIngressHandler from src/app/v1/_lib/ws/ingress-handler.ts (exported at line 521). The WsIngressHandler class implements auth validation, toggle checking, and connection state management, but the real handler is never integrated. The placeholder currently returns "WebSocket ingress not yet initialized" to all messages, bypassing the auth and toggle checks that WsIngressHandler.start() would perform.
Replace the placeholder handler with a call to registerIngressHandler:
import { registerIngressHandler } from "@/app/v1/_lib/ws/ingress-handler";
// Replace lines 25-40 with:
registerIngressHandler(wsManager);Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/index.ts
Line: 24-40
Comment:
**Placeholder handler is never replaced with `registerIngressHandler`**
`src/server/index.ts` registers a placeholder `onConnection` handler (lines 25–40) but never calls `registerIngressHandler` from `src/app/v1/_lib/ws/ingress-handler.ts` (exported at line 521). The `WsIngressHandler` class implements auth validation, toggle checking, and connection state management, but the real handler is never integrated. The placeholder currently returns `"WebSocket ingress not yet initialized"` to all messages, bypassing the auth and toggle checks that `WsIngressHandler.start()` would perform.
Replace the placeholder handler with a call to `registerIngressHandler`:
```ts
import { registerIngressHandler } from "@/app/v1/_lib/ws/ingress-handler";
// Replace lines 25-40 with:
registerIngressHandler(wsManager);
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+497
to
+508
| function extractClientIp(req: IncomingMessage): string { | ||
| const realIp = req.headers["x-real-ip"]; | ||
| if (typeof realIp === "string" && realIp.trim()) return realIp.trim(); | ||
|
|
||
| const forwarded = req.headers["x-forwarded-for"]; | ||
| if (typeof forwarded === "string") { | ||
| const ips = forwarded | ||
| .split(",") | ||
| .map((s) => s.trim()) | ||
| .filter(Boolean); | ||
| if (ips.length > 0) return ips[ips.length - 1]; | ||
| } |
There was a problem hiding this comment.
x-forwarded-for takes last hop instead of first
In extractClientIp at line 507:
if (ips.length > 0) return ips[ips.length - 1];The x-forwarded-for header is structured as client, proxy1, proxy2, …. The first entry is the originating client IP. The last entry is the most recently appended proxy, which can be forged if the ingress doesn't validate the header. This should use the first entry for correct client IP identification:
if (ips.length > 0) return ips[0];Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/ws/ingress-handler.ts
Line: 497-508
Comment:
**`x-forwarded-for` takes last hop instead of first**
In `extractClientIp` at line 507:
```ts
if (ips.length > 0) return ips[ips.length - 1];
```
The `x-forwarded-for` header is structured as `client, proxy1, proxy2, …`. The **first** entry is the originating client IP. The last entry is the most recently appended proxy, which can be forged if the ingress doesn't validate the header. This should use the first entry for correct client IP identification:
```ts
if (ips.length > 0) return ips[0];
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+342
to
+360
| // 5. Execute turn via outbound adapter | ||
| const adapter = new OutboundWsAdapter({ | ||
| providerBaseUrl: provider.url, | ||
| apiKey: provider.key, | ||
| }); | ||
| this.activeAdapter = adapter; | ||
|
|
||
| try { | ||
| const turnResult = await adapter.executeTurn(frame.response as Record<string, unknown>); | ||
|
|
||
| // 6. Relay all events to client via event bridge | ||
| const bridge = new WsEventBridge(); | ||
| for (const event of turnResult.events) { | ||
| bridge.relayEvent( | ||
| this.ws, | ||
| event as { type: string; data: unknown }, | ||
| JSON.stringify(event.data) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Events batched after turn completes — no real-time streaming
adapter.executeTurn() awaits the entire upstream response before returning (it resolves only when a terminal event arrives). The events array collected inside executeTurn() is then replayed to the client in a tight loop after the turn is complete (lines 354–360). This means the client receives no events until the upstream LLM finishes generating, then gets all chunks in a rapid burst — the opposite of real-time streaming.
For a working WS streaming proxy, events must be forwarded to the client as they arrive from the upstream socket. The OutboundWsAdapter needs a callback/sink interface so the ingress-handler can relay each event immediately:
// Example: pass a relay callback into executeTurn
const turnResult = await adapter.executeTurn(
frame.response as Record<string, unknown>,
(event, rawJson) => {
bridge.relayEvent(this.ws, event, rawJson);
}
);And inside OutboundWsAdapter.executeTurn(), call the callback in the "message" handler (around line 186) instead of (or in addition to) pushing to events:
this.ws.on("message", (data: Buffer | string) => {
// ...parse...
const event = { type: eventType, data: parsed };
events.push(event);
onEvent?.(event, raw); // ← relay immediately
// ...terminal check...
});Without this change, every response will appear to hang for its full generation time before any output is visible — making the WebSocket transport behaviorally identical to a slow HTTP request from the client's perspective.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/ws/ingress-handler.ts
Line: 342-360
Comment:
**Events batched after turn completes — no real-time streaming**
`adapter.executeTurn()` awaits the **entire upstream response** before returning (it resolves only when a terminal event arrives). The `events` array collected inside `executeTurn()` is then replayed to the client in a tight loop after the turn is complete (lines 354–360). This means the client receives no events until the upstream LLM finishes generating, then gets all chunks in a rapid burst — the opposite of real-time streaming.
For a working WS streaming proxy, events must be forwarded to the client **as they arrive** from the upstream socket. The `OutboundWsAdapter` needs a callback/sink interface so the `ingress-handler` can relay each event immediately:
```ts
// Example: pass a relay callback into executeTurn
const turnResult = await adapter.executeTurn(
frame.response as Record<string, unknown>,
(event, rawJson) => {
bridge.relayEvent(this.ws, event, rawJson);
}
);
```
And inside `OutboundWsAdapter.executeTurn()`, call the callback in the `"message"` handler (around line 186) instead of (or in addition to) pushing to `events`:
```ts
this.ws.on("message", (data: Buffer | string) => {
// ...parse...
const event = { type: eventType, data: parsed };
events.push(event);
onEvent?.(event, raw); // ← relay immediately
// ...terminal check...
});
```
Without this change, every response will appear to hang for its full generation time before any output is visible — making the WebSocket transport behaviorally identical to a slow HTTP request from the client's perspective.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+97
to
+100
| async executeTurn(requestBody: Record<string, unknown>): Promise<OutboundTurnResult> { | ||
| const events: Array<{ type: string; data: unknown }> = []; | ||
| const wsUrl = toWebSocketUrl(this.opts.providerBaseUrl); | ||
|
|
There was a problem hiding this comment.
Unbounded events array may cause high memory usage on long turns
The events array (line 98) accumulates every upstream server-sent event with no capacity limit. A verbose model (e.g. long chain-of-thought with many response.output_text.delta events) could produce tens of thousands of entries; at scale this results in significant heap pressure per concurrent connection.
Consider either capping the array (e.g. keep only the last N events for debugging purposes, similar to how WsEventBridge uses a ring buffer at line 79) or removing event accumulation from the adapter entirely once event relay is moved to a real-time callback (see comment on ingress-handler.ts). If events need to be replayed, the ring buffer in WsEventBridge is the right place for that bounded storage.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/ws/outbound-adapter.ts
Line: 97-100
Comment:
**Unbounded `events` array may cause high memory usage on long turns**
The `events` array (line 98) accumulates every upstream server-sent event with no capacity limit. A verbose model (e.g. long chain-of-thought with many `response.output_text.delta` events) could produce tens of thousands of entries; at scale this results in significant heap pressure per concurrent connection.
Consider either capping the array (e.g. keep only the last N events for debugging purposes, similar to how `WsEventBridge` uses a ring buffer at line 79) or removing event accumulation from the adapter entirely once event relay is moved to a real-time callback (see comment on `ingress-handler.ts`). If events need to be replayed, the ring buffer in `WsEventBridge` is the right place for that bounded storage.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
♻️ Duplicate comments (4)
src/app/v1/_lib/ws/ingress-handler.ts (4)
501-511:⚠️ Potential issue | 🔴 Critical
x-forwarded-for取值顺序错误，会导致限流/审计使用错误的 IP。标准
x-forwarded-for格式为client, proxy1, proxy2，最左侧是原始客户端 IP。当前代码返回ips[ips.length - 1]（最后一个/最近代理的 IP），而非ips[0]（真实客户端 IP）。这会影响：
- 基于 IP 的限流决策
- 审计日志的准确性
- 任何依赖客户端 IP 的 provider 决策
修复建议
function extractClientIp(req: IncomingMessage): string { const realIp = req.headers["x-real-ip"]; if (typeof realIp === "string" && realIp.trim()) return realIp.trim(); const forwarded = req.headers["x-forwarded-for"]; if (typeof forwarded === "string") { const ips = forwarded .split(",") .map((s) => s.trim()) .filter(Boolean); - if (ips.length > 0) return ips[ips.length - 1]; + if (ips.length > 0) return ips[0]; } return req.socket?.remoteAddress ?? "unknown"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/ingress-handler.ts` around lines 501 - 511, The code incorrectly treats the last entry of the parsed x-forwarded-for header as the client IP; change the logic to return the first non-empty entry instead (use ips[0] rather than ips[ips.length - 1]) so the original client IP is used for rate-limiting/auditing; keep the existing parsing (split on ",", trim, filter(Boolean)) and the fallback to req.socket?.remoteAddress ?? "unknown" unchanged, and update any function that uses the forwarded/ips logic (the variable forwarded and the parsed ips array in this ingress-handler) to return the left-most IP.
109-113:⚠️ Potential issue | 🟠 MajorHeader 类型强转可能导致重复 header 时运行时崩溃。
Node.js 的
IncomingMessage.headers[key]类型是string | string[] | undefined。当客户端发送重复的x-api-key或x-goog-api-keyheader 时，值会变成string[]。此时as string强转不会在运行时生效，后续extractApiKeyFromHeaders内部调用.trim()会抛出TypeError。修复建议：在传入前规范化 header 值
// 2. Authenticate using request headers +const xApiKey = this.req.headers["x-api-key"]; +const xGoogApiKey = this.req.headers["x-goog-api-key"]; const apiKey = extractApiKeyFromHeaders({ authorization: this.req.headers.authorization ?? null, - "x-api-key": (this.req.headers["x-api-key"] as string) ?? null, - "x-goog-api-key": (this.req.headers["x-goog-api-key"] as string) ?? null, + "x-api-key": typeof xApiKey === "string" ? xApiKey : null, + "x-goog-api-key": typeof xGoogApiKey === "string" ? xGoogApiKey : null, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/ingress-handler.ts` around lines 109 - 113, The headers for "x-api-key" and "x-goog-api-key" are being force-cast to string which will throw if the incoming header is a string[]; before calling extractApiKeyFromHeaders, normalize these header values from this.req.headers by checking for undefined, string, or string[] and convert to a single trimmed string or null (e.g., pick the first value or join with ',' then trim) so extractApiKeyFromHeaders always receives string|null and cannot trigger a .trim() TypeError; update the call site that builds the object passed to extractApiKeyFromHeaders to perform this normalization for "x-api-key" and "x-goog-api-key" (and optionally authorization) instead of using as string casts.
144-148:⚠️ Potential issue | 🟡 MinorDebug 日志包含 PII（用户名和客户端 IP）。
userName和clientIp属于用户可识别信息，记录在 WS 连接路径的日志中会增加泄露风险。建议：
- 移除
userName，仅保留userId- 对 IP 进行截断或哈希处理
修复建议
logger.debug("[WsIngress] Authenticated", { userId: authResult.user.id, - userName: authResult.user.name, - clientIp: this.ip, + // 如需 IP 用于调试，考虑使用哈希或截断 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/ingress-handler.ts` around lines 144 - 148, The debug log in WsIngress currently writes PII (authResult.user.name and this.ip); modify the logger.debug call in ingress-handler.ts (the call referencing logger.debug and authResult.user) to remove userName and only include authResult.user.id, and replace this.ip with a non-PII form (e.g., truncate or hash the IP before logging — implement a helper function like truncateIp(ip) or hashIp(ip) and call it where this.ip is used) so the log contains userId and a redacted/hashed client IP instead of raw PII.
521-531:⚠️ Potential issue | 🟠 Major
registerIngressHandler的 async 回调可能产生未处理的 Promise rejection。
wsManager.onConnection()不会 await 回调返回值（参见 ws-manager.ts:56-65）。如果handler.start()抛出异常（例如isResponsesWebSocketEnabled()的缓存/数据库错误），会导致未处理的 Promise rejection，可能使进程崩溃或留下未清理的连接。修复建议：改用非 async 回调并显式 .catch()
export function registerIngressHandler( wsManager: import("@/server/ws-manager").WsManager, options?: IngressHandlerOptions ): void { - wsManager.onConnection(async (ws, req) => { + wsManager.onConnection((ws, req) => { const handler = new WsIngressHandler(ws, req, options); - const ok = await handler.start(); - if (!ok) { - logger.debug("[WsIngress] Connection rejected during init"); - } + handler.start().then((ok) => { + if (!ok) { + logger.debug("[WsIngress] Connection rejected during init"); + } + }).catch((err) => { + logger.error("[WsIngress] Connection init failed", { error: err }); + ws.close(1011, "Internal server error"); + }); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/ws/ingress-handler.ts` around lines 521 - 531, registerIngressHandler uses an async callback for wsManager.onConnection which can produce unhandled Promise rejections if handler.start() throws; change the callback to a non-async function and call handler.start().catch(...) (or attach .then(...) to handle the boolean return and .catch to log and clean up) so all errors are handled. Specifically, update the wsManager.onConnection callback that creates new WsIngressHandler(ws, req, options) to invoke handler.start() with an explicit .catch that logs the error (use logger.error or logger.debug) and ensures any necessary cleanup/close of ws to avoid leaking connections; reference WsIngressHandler, registerIngressHandler, wsManager.onConnection, and handler.start when locating the change.
🧹 Nitpick comments (1)
tests/unit/ws/ingress-handler.test.ts (1)
224-241: 建议补充x-forwarded-for的 IP 提取测试。当前测试覆盖了
x-real-ip优先级，但缺少对x-forwarded-for多 IP 情况的测试。鉴于当前实现存在取值顺序 bug（返回最后一个 IP 而非第一个），建议添加测试用例以验证修复后的行为：test("extracts first IP from x-forwarded-for chain", async () => { const ws = createMockWs(); const handler = new WsIngressHandler( ws as any, createMockReq({ "x-forwarded-for": "203.0.113.50, 10.0.0.1, 192.168.1.1" }) ); await handler.start(); expect(handler.clientIp).toBe("203.0.113.50"); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/ws/ingress-handler.test.ts` around lines 224 - 241, Add a unit test that verifies WsIngressHandler extracts the first IP from an x-forwarded-for header chain: create a mock socket with createMockWs(), construct a WsIngressHandler with createMockReq({ "x-forwarded-for": "203.0.113.50, 10.0.0.1, 192.168.1.1" }), call handler.start(), and assert handler.clientIp === "203.0.113.50"; this will catch the current bug where the last IP is returned instead of the first.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/app/v1/_lib/ws/ingress-handler.ts`:
- Around line 501-511: The code incorrectly treats the last entry of the parsed
x-forwarded-for header as the client IP; change the logic to return the first
non-empty entry instead (use ips[0] rather than ips[ips.length - 1]) so the
original client IP is used for rate-limiting/auditing; keep the existing parsing
(split on ",", trim, filter(Boolean)) and the fallback to
req.socket?.remoteAddress ?? "unknown" unchanged, and update any function that
uses the forwarded/ips logic (the variable forwarded and the parsed ips array in
this ingress-handler) to return the left-most IP.
- Around line 109-113: The headers for "x-api-key" and "x-goog-api-key" are
being force-cast to string which will throw if the incoming header is a
string[]; before calling extractApiKeyFromHeaders, normalize these header values
from this.req.headers by checking for undefined, string, or string[] and convert
to a single trimmed string or null (e.g., pick the first value or join with ','
then trim) so extractApiKeyFromHeaders always receives string|null and cannot
trigger a .trim() TypeError; update the call site that builds the object passed
to extractApiKeyFromHeaders to perform this normalization for "x-api-key" and
"x-goog-api-key" (and optionally authorization) instead of using as string
casts.
- Around line 144-148: The debug log in WsIngress currently writes PII
(authResult.user.name and this.ip); modify the logger.debug call in
ingress-handler.ts (the call referencing logger.debug and authResult.user) to
remove userName and only include authResult.user.id, and replace this.ip with a
non-PII form (e.g., truncate or hash the IP before logging — implement a helper
function like truncateIp(ip) or hashIp(ip) and call it where this.ip is used) so
the log contains userId and a redacted/hashed client IP instead of raw PII.
- Around line 521-531: registerIngressHandler uses an async callback for
wsManager.onConnection which can produce unhandled Promise rejections if
handler.start() throws; change the callback to a non-async function and call
handler.start().catch(...) (or attach .then(...) to handle the boolean return
and .catch to log and clean up) so all errors are handled. Specifically, update
the wsManager.onConnection callback that creates new WsIngressHandler(ws, req,
options) to invoke handler.start() with an explicit .catch that logs the error
(use logger.error or logger.debug) and ensures any necessary cleanup/close of ws
to avoid leaking connections; reference WsIngressHandler,
registerIngressHandler, wsManager.onConnection, and handler.start when locating
the change.
---
Nitpick comments:
In `@tests/unit/ws/ingress-handler.test.ts`:
- Around line 224-241: Add a unit test that verifies WsIngressHandler extracts
the first IP from an x-forwarded-for header chain: create a mock socket with
createMockWs(), construct a WsIngressHandler with createMockReq({
"x-forwarded-for": "203.0.113.50, 10.0.0.1, 192.168.1.1" }), call
handler.start(), and assert handler.clientIp === "203.0.113.50"; this will catch
the current bug where the last IP is returned instead of the first.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f010be5c-fa8c-443d-b768-251cd717faea
📒 Files selected for processing (4)
src/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/ws/ingress-handler.tstests/unit/ws/ingress-handler-integration.test.tstests/unit/ws/ingress-handler.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/v1/responsesWebSocket mode to Claude Code Hub with global system toggle, same-port HTTP+WS server, outbound WS adapter, inbound ingress with delayed bridging, billing/observability parity, and provider WS testingRelated PRs:
src/server/andsrc/app/v1/_lib/ws/organizationWave 1: Foundation
enableResponsesWebSocketsystem toggle (DB schema, types, cache, actions, settings UI, 5 locales)response.create, terminal events, error frames)wson/v1/responsesws_fallbackreasonWave 2: Core Pipeline
response.completed/failed/incomplete)Wave 3: Parity + Product Surface
WsTestStatus) with transport badge and 5-language i18nTest plan
bun run typecheckpassesbun run lintpasses (warnings only, pre-existing)bun run testpasses (3,775 tests, 0 failures)bun run buildpasses (production build with custom server entry)bunx vitest run tests/unit/lib/ws/ tests/unit/ws/ tests/unit/server/ tests/unit/proxy/transport-classifier.test.ts tests/unit/provider-testing/(241 tests)/v1/responsesPOST route unaffectedenableResponsesWebSocketin settings UI, verify persistenceDescription enhanced by Claude AI
Greptile Summary
This PR adds a full OpenAI
/v1/responsesWebSocket mode to Claude Code Hub, spanning a custom Node.js server, inbound ingress handler, outbound WS adapter, event bridge, billing parity, session continuity, and provider testing UI. The foundation (server setup, Zod schemas, billing parity, session continuity) is well-engineered with comprehensive test coverage (241 WS-specific tests).However, there are several critical issues that must be addressed:
Critical logic issues:
toWebSocketUrlincorrectly appends/v1/responsesto provider base URLs that already contain/v1, resulting in URLs likewss://api.openai.com/v1/v1/responses. This breaks connectivity for the most common provider configuration (OpenAI).OutboundWsAdaptercollects ALL upstream events in an array, then resolves only when the terminal event arrives. The ingress handler then replays this array to the client after the turn completes. This means clients receive zero events during generation, then get everything in a burst — defeating the primary purpose of WebSocket streaming. A real-time callback mechanism is needed to forward events as they arrive.registerIngressHandler. Auth validation, toggle checking, and connection state management are bypassed.extractClientIpuses the last IP from thex-forwarded-forheader instead of the first, reducing client identification accuracy when behind proxies.Secondary concern:
eventsarray inOutboundWsAdapterhas no capacity limit, which could cause memory pressure on verbose model responses at scale.Confidence Score: 1/5
Sequence Diagram
sequenceDiagram participant C as Client (WS) participant I as WsIngressHandler participant G as GuardPipeline participant T as TransportClassifier participant O as OutboundWsAdapter participant U as Upstream Provider (WS) participant B as WsEventBridge participant DB as Database (billing) C->>I: WS Upgrade (Authorization header) I->>I: isResponsesWebSocketEnabled() I->>I: validateApiKeyAndGetUser() I-->>C: Connection Accepted C->>I: response.create frame I->>I: state = "processing" I->>G: pipeline.run(session) [model, provider, messageContext] G-->>I: guard passed I->>T: classifyTransport(session, provider) T-->>I: transport = "websocket" I->>O: executeTurn(requestBody) O->>U: WS Upgrade (wss://provider/v1/responses) U-->>O: Connection Open O->>U: response.create frame U->>O: streaming events (tokens, deltas...) Note over O,U: ⚠️ Events collected in memory array U->>O: response.completed (terminal) O-->>I: OutboundTurnResult {events[], completed} Note over I,O: ⚠️ Critical: All events arrive at once AFTER turn completes loop for each event in turnResult.events I->>B: bridge.relayEvent(clientWs, event) B-->>C: send event (batched, not real-time) end I->>DB: updateMessageRequestCost() I->>DB: updateMessageRequestDetails() I->>I: state = "waiting" C->>I: (next response.create or close)Last reviewed commit: 30fb85c