feat: add response input rectifier for /v1/responses#888
Conversation
OpenAI Responses API input field supports string shortcut, single object,
and array formats. This rectifier normalizes non-array input to standard
array format before the guard pipeline, with audit trail via special
settings persisted to message_requests.
- Normalize string input to [{role:"user",content:[{type:"input_text",text}]}]
- Wrap single object input (with role/type) into array
- Convert empty string to empty array
- Passthrough array input unchanged
- Default enabled, toggleable via system settings UI
- Broadened format-mapper body detection for string/object input
- Full i18n support (en, zh-CN, zh-TW, ja, ru)
- 14 unit tests covering all normalization paths
📝 WalkthroughWalkthrough该PR新增响应输入整流器功能：在系统设置添加开关，并实现 /v1/responses 输入规范化逻辑、审计记录、类型/验证、UI 开关、多语言配置与数据库迁移。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 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 |
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 introduces a new feature that enhances the robustness and flexibility of the 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
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a great pull request that adds a valuable feature for normalizing /v1/responses input. The changes are comprehensive, touching all necessary parts of the codebase from the database to the UI. The inclusion of i18n support, auditing, and thorough unit tests is commendable. I have a couple of minor suggestions to improve code clarity in the new logic, but overall this is excellent work.
| (typeof requestBody.input === "object" && | ||
| requestBody.input !== null && | ||
| ("role" in (requestBody.input as object) || "type" in (requestBody.input as object))) |
There was a problem hiding this comment.
The type assertion as object is redundant in this context. The typeof requestBody.input === 'object' check already narrows the type sufficiently for the in operator to work correctly. Removing the unnecessary cast will make the code cleaner and more readable.
| (typeof requestBody.input === "object" && | |
| requestBody.input !== null && | |
| ("role" in (requestBody.input as object) || "type" in (requestBody.input as object))) | |
| (typeof requestBody.input === "object" && | |
| requestBody.input !== null && | |
| ("role" in requestBody.input || "type" in requestBody.input)) |
| return { | ||
| applied: false, | ||
| action: "passthrough", | ||
| originalType: input === undefined || input === null ? "other" : "other", |
There was a problem hiding this comment.
The conditional expression input === undefined || input === null ? "other" : "other" is redundant because it will always evaluate to "other". Simplifying this will improve code clarity.
| originalType: input === undefined || input === null ? "other" : "other", | |
| originalType: "other", |
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return { | ||
| applied: false, | ||
| action: "passthrough", | ||
| originalType: input === undefined || input === null ? "other" : "other", |
There was a problem hiding this comment.
Redundant ternary — both branches always return "other"
The conditional expression on line 72 evaluates to "other" regardless of which branch is taken:
originalType: input === undefined || input === null ? "other" : "other",The false branch is reached when input is an object that didn't match Case 3 (no role or type key), a number, or a boolean — all of which the type system maps to "other" anyway. This dead ternary obscures intent and could confuse future maintainers. Consider simplifying:
| originalType: input === undefined || input === null ? "other" : "other", | |
| originalType: "other", |
If the intent was to distinguish null/undefined from other unexpected types (e.g., return "object" for unrecognized plain objects), the ResponseInputRectifierAction type and tests would need to be updated accordingly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/response-input-rectifier.ts
Line: 72
Comment:
**Redundant ternary — both branches always return `"other"`**
The conditional expression on line 72 evaluates to `"other"` regardless of which branch is taken:
```typescript
originalType: input === undefined || input === null ? "other" : "other",
```
The false branch is reached when `input` is an object that didn't match Case 3 (no `role` or `type` key), a number, or a boolean — all of which the type system maps to `"other"` anyway. This dead ternary obscures intent and could confuse future maintainers. Consider simplifying:
```suggestion
originalType: "other",
```
If the intent was to distinguish `null`/`undefined` from other unexpected types (e.g., return `"object"` for unrecognized plain objects), the `ResponseInputRectifierAction` type and tests would need to be updated accordingly.
How can I resolve this? If you propose a fix, please make it concise.| import { describe, expect, it } from "vitest"; | ||
| import { rectifyResponseInput } from "@/app/v1/_lib/proxy/response-input-rectifier"; | ||
|
|
There was a problem hiding this comment.
No test coverage for normalizeResponseInput (the async entry-point)
The test file only exercises rectifyResponseInput directly. The public entry-point normalizeResponseInput — which is what proxy-handler.ts actually calls — has zero test coverage. This function:
- Reads
getCachedSystemSettings()and checks theenableResponseInputRectifierflag - Calls
rectifyResponseInputand conditionally callssession.addSpecialSetting() - Logs via
logger.info
A future change to the settings-gate logic or the audit-trail path (e.g. calling addSpecialSetting for the passthrough case too) could silently break without any test catching it. Consider adding at least two tests:
- Feature disabled: mock
getCachedSystemSettingsto returnenableResponseInputRectifier: false, send a stringinput, and assert the body is unchanged and no special setting is added. - Feature enabled: mock settings to return
true, send a stringinput, and assert both the normalization and theaddSpecialSettingcall happen correctly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/proxy/response-input-rectifier.test.ts
Line: 1-3
Comment:
**No test coverage for `normalizeResponseInput` (the async entry-point)**
The test file only exercises `rectifyResponseInput` directly. The public entry-point `normalizeResponseInput` — which is what `proxy-handler.ts` actually calls — has zero test coverage. This function:
- Reads `getCachedSystemSettings()` and checks the `enableResponseInputRectifier` flag
- Calls `rectifyResponseInput` and conditionally calls `session.addSpecialSetting()`
- Logs via `logger.info`
A future change to the settings-gate logic or the audit-trail path (e.g. calling `addSpecialSetting` for the `passthrough` case too) could silently break without any test catching it. Consider adding at least two tests:
1. **Feature disabled**: mock `getCachedSystemSettings` to return `enableResponseInputRectifier: false`, send a string `input`, and assert the body is unchanged and no special setting is added.
2. **Feature enabled**: mock settings to return `true`, send a string `input`, and assert both the normalization and the `addSpecialSetting` call happen correctly.
How can I resolve this? If you propose a fix, please make it concise.| // input 支持数组、字符串简写、单对象三种格式 | ||
| if ( | ||
| Array.isArray(requestBody.input) || | ||
| typeof requestBody.input === "string" || | ||
| (typeof requestBody.input === "object" && | ||
| requestBody.input !== null && | ||
| ("role" in (requestBody.input as object) || "type" in (requestBody.input as object))) | ||
| ) { | ||
| return "response"; | ||
| } |
There was a problem hiding this comment.
Body-based detection now classifies any string input as Response API
With this PR, detectClientFormat (the body-based fallback) returns "response" for any request body whose input field is a plain string or a top-level object — not just arrays.
This broadening is intentional and safe when the endpoint is known (because detectFormatByEndpoint takes priority and returns early), but it has a subtle risk on unknown endpoints: the OpenAI Embeddings API (/v1/embeddings) also uses a top-level input field that can be a string. If such a request reaches this fallback (e.g., via a generic catch-all route), it would be misclassified as "response" and passed through the Response API rectifier / converters.
Consider adding an entry for /v1/embeddings to detectFormatByEndpoint to ensure the endpoint-first path handles it, preventing the body-based fallback from misrouting it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/format-mapper.ts
Line: 124-133
Comment:
**Body-based detection now classifies any string `input` as Response API**
With this PR, `detectClientFormat` (the body-based *fallback*) returns `"response"` for any request body whose `input` field is a plain string or a top-level object — not just arrays.
This broadening is intentional and safe when the endpoint is known (because `detectFormatByEndpoint` takes priority and returns early), but it has a subtle risk on *unknown* endpoints: the OpenAI Embeddings API (`/v1/embeddings`) also uses a top-level `input` field that can be a string. If such a request reaches this fallback (e.g., via a generic catch-all route), it would be misclassified as `"response"` and passed through the Response API rectifier / converters.
Consider adding an entry for `/v1/embeddings` to `detectFormatByEndpoint` to ensure the endpoint-first path handles it, preventing the body-based fallback from misrouting it.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/app/v1/_lib/proxy/response-input-rectifier.ts (1)
68-74: 冗余的三元表达式。第 72 行的三元表达式
input === undefined || input === null ? "other" : "other"两个分支返回相同的值"other"，属于冗余代码。建议简化
// Case 4: undefined/null/其他 -- passthrough，让下游处理错误 return { applied: false, action: "passthrough", - originalType: input === undefined || input === null ? "other" : "other", + originalType: "other", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/response-input-rectifier.ts` around lines 68 - 74, The ternary in the return object inside response-input-rectifier (the block returning { applied: false, action: "passthrough", originalType: input === undefined || input === null ? "other" : "other" }) is redundant; replace the whole ternary with the literal "other" for originalType (i.e., originalType: "other") so the return becomes explicit and simpler.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/ru/settings/config.json`:
- Around line 58-59: The description text for enableResponseInputRectifier is
too broad: update the value of "enableResponseInputRectifierDesc" to explicitly
state that only non-array single input objects that include a "role" field are
normalized (not arbitrary single objects), so administrators understand the
rectifier only converts single objects with a role into the standard array
format for /v1/responses.
In `@messages/zh-CN/settings/config.json`:
- Around line 47-48: Update the description string for
enableResponseInputRectifierDesc so it precisely states the rectifier normalizes
"string shorthand or single objects that include a role" (i.e., "字符串简写或带 role
的单对象") rather than implying all single objects; locate the
enableResponseInputRectifierDesc entry and replace the current text with the
corrected wording so users understand only objects containing a role are wrapped
into an array.
In `@messages/zh-TW/settings/config.json`:
- Around line 58-59: The description string for enableResponseInputRectifier is
too broad; update the value of enableResponseInputRectifierDesc to explicitly
state that only string shorthand or single objects that include a role will be
normalized (not every single-object input). Locate the localization key
"enableResponseInputRectifierDesc" and change the wording to something like "自動將
/v1/responses 請求中的字串簡寫或帶 role 的單物件規範化為標準陣列格式後再處理（預設開啟）。" ensuring the key
"enableResponseInputRectifier" remains unchanged.
In `@src/app/v1/_lib/proxy-handler.ts`:
- Around line 53-56: The call to normalizeResponseInput(session) performs I/O
(reads system settings) and must not run before the GuardPipeline/ auth guard;
either convert the normalization into a new guard and insert it into the
GuardPipeline immediately after the auth guard (and before any guards that
expect normalized inputs), or refactor normalizeResponseInput into a pure
in-memory transform by having the relevant feature flag/value injected earlier
(so session.originalFormat can be normalized without I/O). Update usages: remove
the direct call in the proxy handler (the session.originalFormat branch) and
instead implement the new Guard (or pure transform) so normalization happens
inside the pipeline after auth and before downstream guards that depend on
normalized input.
In `@src/app/v1/_lib/proxy/format-mapper.ts`:
- Around line 124-131: The current fallback in detectClientFormat() in
format-mapper.ts incorrectly treats any requestBody.input that is a string or
single-object (with role/type) as the Responses format even for unknown
endpoints; restrict this by gating the shorthand detection so it only classifies
as "response" when the endpoint is explicitly known to be /v1/responses or when
a pathname argument is provided and matches that route. Update the detection
logic around requestBody.input to require either an explicit responses endpoint
flag or pathname === '/v1/responses' (or an explicit format hint) before
treating string/object shorthands as Responses, and ensure any callers pass
pathname or the endpoint-identified flag into detectClientFormat().
In `@src/repository/system-config.ts`:
- Around line 428-431: The returning clause in updateSystemSettings is missing
the enableBillingHeaderRectifier field which exists in fullSelection; update the
RETURNING list in the updateSystemSettings SQL/method to include
enableBillingHeaderRectifier (place it after enableThinkingBudgetRectifier as
suggested) so the updated object includes that field and matches fullSelection.
---
Nitpick comments:
In `@src/app/v1/_lib/proxy/response-input-rectifier.ts`:
- Around line 68-74: The ternary in the return object inside
response-input-rectifier (the block returning { applied: false, action:
"passthrough", originalType: input === undefined || input === null ? "other" :
"other" }) is redundant; replace the whole ternary with the literal "other" for
originalType (i.e., originalType: "other") so the return becomes explicit and
simpler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c73021a-d5cd-4a68-ab73-0782e05a7a67
📒 Files selected for processing (23)
drizzle/0079_easy_zeigeist.sqldrizzle/meta/0079_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/settings/config.jsonmessages/ja/settings/config.jsonmessages/ru/settings/config.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/settings/config.jsonsrc/actions/system-config.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/v1/_lib/proxy-handler.tssrc/app/v1/_lib/proxy/format-mapper.tssrc/app/v1/_lib/proxy/response-input-rectifier.tssrc/drizzle/schema.tssrc/lib/config/system-settings-cache.tssrc/lib/utils/special-settings.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/system-config.tssrc/types/special-settings.tssrc/types/system-config.tstests/unit/proxy/response-input-rectifier.test.ts
messages/ru/settings/config.json
Outdated
| "enableResponseInputRectifier": "Включить исправление Response Input", | ||
| "enableResponseInputRectifierDesc": "Автоматически нормализует не-массивные input (строковые сокращения или одиночные объекты) в запросах /v1/responses в стандартный формат массива перед обработкой (включено по умолчанию).", |
There was a problem hiding this comment.
这里的功能描述比实现更宽。
文案写成了“单对象”都会被规范化，但实际规则只覆盖带 role 的单对象。建议把说明收窄，否则会让管理员误以为任意对象输入都能被兼容。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ru/settings/config.json` around lines 58 - 59, The description text
for enableResponseInputRectifier is too broad: update the value of
"enableResponseInputRectifierDesc" to explicitly state that only non-array
single input objects that include a "role" field are normalized (not arbitrary
single objects), so administrators understand the rectifier only converts single
objects with a role into the standard array format for /v1/responses.
| // Response API input rectifier: normalize non-array input before guard pipeline | ||
| if (session.originalFormat === "response") { | ||
| await normalizeResponseInput(session); | ||
| } |
There was a problem hiding this comment.
不要在 GuardPipeline 之前做带 I/O 的整流。
normalizeResponseInput() 这里不是纯同步变换，它会先读系统设置。这样一来，/v1/responses 请求会在 auth guard 之前先走一段配置读取路径；冷缓存或缓存刷新时，未鉴权流量也会先打到这条依赖链上。整流逻辑本身没问题，但放在 pipeline 外会把原本统一的 guard 边界打散。
更稳妥的做法是：把它做成一个 guard（放到 auth 之后、后续依赖标准化输入的 guard 之前），或者把开关值以无 I/O 的方式提前注入，再在这里执行纯内存整流。
Based on learnings: src/app/v1/_lib/proxy/**/*.{ts,tsx}: The proxy pipeline processes requests through a GuardPipeline with sequential guards: auth, sensitive, client, model, version, probe, session, warmup, requestFilter, rateLimit, provider, providerRequestFilter, messageContext
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/v1/_lib/proxy-handler.ts` around lines 53 - 56, The call to
normalizeResponseInput(session) performs I/O (reads system settings) and must
not run before the GuardPipeline/ auth guard; either convert the normalization
into a new guard and insert it into the GuardPipeline immediately after the auth
guard (and before any guards that expect normalized inputs), or refactor
normalizeResponseInput into a pure in-memory transform by having the relevant
feature flag/value injected earlier (so session.originalFormat can be normalized
without I/O). Update usages: remove the direct call in the proxy handler (the
session.originalFormat branch) and instead implement the new Guard (or pure
transform) so normalization happens inside the pipeline after auth and before
downstream guards that depend on normalized input.
| // input 支持数组、字符串简写、单对象三种格式 | ||
| if ( | ||
| Array.isArray(requestBody.input) || | ||
| typeof requestBody.input === "string" || | ||
| (typeof requestBody.input === "object" && | ||
| requestBody.input !== null && | ||
| ("role" in (requestBody.input as object) || "type" in (requestBody.input as object))) | ||
| ) { |
There was a problem hiding this comment.
不要把未知端点仅凭 input 识别成 Responses 格式。
这里把任意 input: string / input: { type | role } 都判成 "response"，而 detectClientFormat() 本身又是“端点未识别时”的兜底逻辑。这样会把其他同样使用 input 字段的未知端点误送进 /v1/responses 分支，超出了这次“仅整流 /v1/responses”的范围。
建议把“字符串简写 / 单对象简写”的识别收敛到端点已明确为 /v1/responses 的场景；如果要在 body fallback 里支持，至少需要把 pathname 一并传进来做 gated detection。
可行的收敛方式
-export function detectClientFormat(requestBody: Record<string, unknown>): ClientFormat {
+export function detectClientFormat(
+ requestBody: Record<string, unknown>,
+ pathname?: string,
+): ClientFormat {
+ const normalizedPath = pathname?.split("?")[0].replace(/\/$/, "");
+ const isResponsesEndpoint = normalizedPath === "/v1/responses";
+
// 3. 检测 Response API (Codex) 格式
// input 支持数组、字符串简写、单对象三种格式
if (
Array.isArray(requestBody.input) ||
- typeof requestBody.input === "string" ||
- (typeof requestBody.input === "object" &&
- requestBody.input !== null &&
- ("role" in (requestBody.input as object) || "type" in (requestBody.input as object)))
+ (isResponsesEndpoint &&
+ (typeof requestBody.input === "string" ||
+ (typeof requestBody.input === "object" &&
+ requestBody.input !== null &&
+ ("role" in (requestBody.input as object) ||
+ "type" in (requestBody.input as object)))))
) {
return "response";
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/v1/_lib/proxy/format-mapper.ts` around lines 124 - 131, The current
fallback in detectClientFormat() in format-mapper.ts incorrectly treats any
requestBody.input that is a string or single-object (with role/type) as the
Responses format even for unknown endpoints; restrict this by gating the
shorthand detection so it only classifies as "response" when the endpoint is
explicitly known to be /v1/responses or when a pathname argument is provided and
matches that route. Update the detection logic around requestBody.input to
require either an explicit responses endpoint flag or pathname ===
'/v1/responses' (or an explicit format hint) before treating string/object
shorthands as Responses, and ensure any callers pass pathname or the
endpoint-identified flag into detectClientFormat().
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR implements a response input rectifier that normalizes the /v1/responses input field from string shortcut or single object format into the standard array format. The implementation follows existing patterns in the codebase (similar to thinking-signature-rectifier and billing-header-rectifier) and includes proper integration with system settings, audit logging via special_settings, and full i18n support.
The core normalization logic is correct and well-tested with 14 unit tests. The integration point in proxy-handler.ts is properly placed before the guard pipeline. The database migration is non-destructive.
PR Size: XL
- Lines changed: 4,292
- Files changed: 23
- Labels: size/XL, enhancement, area:core, area:i18n
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Proper (fail-open cache, global error handler)
- Type safety - Clean (well-typed special settings)
- Documentation accuracy - Clean
- Test coverage - Adequate (14 tests for core logic)
- Code clarity - Good
Notes (Not Blocking)
-
Dead code ternary (response-input-rectifier.ts:72): The ternary
input === undefined || input === null ? "other" : "other"always returns "other" regardless of condition. Both branches are identical, making the condition redundant. This is cosmetic and doesn't affect functionality. -
Test coverage gap: The async entry point
normalizeResponseInput()has no direct test, but its functionality (settings check, audit trail) is covered indirectly through the 14rectifyResponseInputunit tests. -
Format detection edge case: The broadened body-based detection in format-mapper.ts could match string
inputon unknown endpoints (not in detectFormatByEndpoint) as "response" format. This was noted in the PR description and only affects non-standard/custom endpoints - the main/v1/responsespath works correctly.
Automated review by Claude AI
| // 3. 检测 Response API (Codex) 格式 | ||
| if (Array.isArray(requestBody.input)) { | ||
| // input 支持数组、字符串简写、单对象三种格式 | ||
| if ( |
There was a problem hiding this comment.
[MEDIUM] [TEST-INCOMPLETE] detectClientFormat() expanded Response API detection has no unit coverage
Evidence (src/app/v1/_lib/proxy/format-mapper.ts:125):
if (
Array.isArray(requestBody.input) ||
typeof requestBody.input === "string" ||
(typeof requestBody.input === "object" &&
requestBody.input !== null &&
("role" in (requestBody.input as object) || "type" in (requestBody.input as object)))
) {
return "response";
}Guideline (CLAUDE.md): 2. **Test Coverage** - All new features must have unit test coverage of at least 80%
Why this is a problem: This logic affects fallback request-format routing. Without unit tests for the new string/single-object branches, a future refactor can break Response API clients without detection.
Suggested fix (add a focused unit test file, e.g. tests/unit/proxy/format-mapper-detect-client-format.test.ts):
import { describe, expect, it } from "vitest";
import { detectClientFormat } from "@/app/v1/_lib/proxy/format-mapper";
describe("detectClientFormat - response input variants", () => {
it("detects response when input is a string", () => {
expect(detectClientFormat({ input: "hi" })).toBe("response");
});
it("detects response when input is a single message object (role)", () => {
expect(detectClientFormat({ input: { role: "user", content: [] } })).toBe("response");
});
it("detects response when input is a single item object (type)", () => {
expect(detectClientFormat({ input: { type: "input_text", text: "hi" } })).toBe("response");
});
});
| @@ -0,0 +1,153 @@ | |||
| import { describe, expect, it } from "vitest"; | |||
| import { rectifyResponseInput } from "@/app/v1/_lib/proxy/response-input-rectifier"; | |||
There was a problem hiding this comment.
[MEDIUM] [TEST-INCOMPLETE] normalizeResponseInput() (settings gate + audit) is not covered
Evidence:
tests/unit/proxy/response-input-rectifier.test.ts:2only imports/testsrectifyResponseInput()src/app/v1/_lib/proxy/response-input-rectifier.ts:80introducesnormalizeResponseInput()which gates the feature via system settings and records the special-setting audit
Guideline (CLAUDE.md): 2. **Test Coverage** - All new features must have unit test coverage of at least 80%
Why this is a problem: The core normalization logic is tested, but regressions in the settings toggle or audit path (e.g. failing to add the special setting when normalization happens) would not be caught.
Suggested fix (extend this test file to cover enabled/disabled branches):
import { describe, expect, it, vi } from "vitest";
import type { ProxySession } from "@/app/v1/_lib/proxy/session";
import { getCachedSystemSettings } from "@/lib/config/system-settings-cache";
import { normalizeResponseInput } from "@/app/v1/_lib/proxy/response-input-rectifier";
vi.mock("@/lib/config/system-settings-cache", () => ({ getCachedSystemSettings: vi.fn() }));
const getCachedSystemSettingsMock = vi.mocked(getCachedSystemSettings);
test("normalizes string input + audits when enabled", async () => {
getCachedSystemSettingsMock.mockResolvedValue({ enableResponseInputRectifier: true } as any);
const audits: unknown[] = [];
const session = {
request: { message: { input: "hello" } },
sessionId: "s1",
addSpecialSetting: (s: unknown) => audits.push(s),
} as unknown as ProxySession;
await normalizeResponseInput(session);
expect((session.request.message as any).input).toEqual([
{ role: "user", content: [{ type: "input_text", text: "hello" }] },
]);
expect(audits).toHaveLength(1);
});
test("skips normalization when disabled", async () => {
getCachedSystemSettingsMock.mockResolvedValue({ enableResponseInputRectifier: false } as any);
const audits: unknown[] = [];
const session = {
request: { message: { input: "hello" } },
sessionId: "s1",
addSpecialSetting: (s: unknown) => audits.push(s),
} as unknown as ProxySession;
await normalizeResponseInput(session);
expect((session.request.message as any).input).toBe("hello");
expect(audits).toEqual([]);
});
There was a problem hiding this comment.
Code Review Summary
This PR introduces a new Response API input rectifier plus a system-setting toggle and i18n strings. The main risk I see is regression exposure: two critical-path behavior changes are not covered by unit tests.
PR Size: XL
- Lines changed: 4292
- Files changed: 23
- Split suggestions:
- PR 1: core proxy logic + tests (
src/app/v1/_lib/proxy-handler.ts,src/app/v1/_lib/proxy/*) - PR 2: system settings plumbing + UI + i18n (
src/actions/*,src/app/[locale]/*,messages/*,src/lib/*,src/types/*) - PR 3: DB schema/migrations/meta (
src/drizzle/schema.ts,drizzle/*)
- PR 1: core proxy logic + tests (
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 2 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- (Confidence 80) [MEDIUM] [TEST-INCOMPLETE] Missing unit coverage for new
detectClientFormat()Response API branches (src/app/v1/_lib/proxy/format-mapper.ts:125) - (Confidence 80) [MEDIUM] [TEST-INCOMPLETE]
normalizeResponseInput()settings gate + audit path is untested (tests/unit/proxy/response-input-rectifier.test.ts:2)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#888(feat: add response input rectifier for /v1/responses); computed XL (4292 LOC changed, 23 files) and applied labelsize/XL. - Posted 2 inline review comments (diff-only):
src/app/v1/_lib/proxy/format-mapper.ts:125— missing unit coverage for newdetectClientFormat()Response API branches.tests/unit/proxy/response-input-rectifier.test.ts:2—normalizeResponseInput()settings gate + audit path isn’t covered by tests.
- Submitted the required review summary via
gh pr review --comment.
- Remove redundant ternary in rectifyResponseInput (both branches "other") - Revert body-based detection broadening in detectClientFormat to prevent misrouting /v1/embeddings and other endpoints with string input fields - Add missing enableBillingHeaderRectifier to returning clause (pre-existing) - Refine i18n descriptions to specify "single message object" (5 locales) - Add 4 normalizeResponseInput tests covering settings gate and audit trail
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| // 3. 检测 Response API (Codex) 格式 | ||
| // 仅通过 input 数组识别；字符串/单对象简写由 response-input-rectifier 在端点确认后规范化 | ||
| if (Array.isArray(requestBody.input)) { | ||
| return "response"; | ||
| } |
There was a problem hiding this comment.
PR description contradicts the actual code change
The PR summary states "Broadened detectClientFormat() body-based fallback to recognize string and single-object input as Response API format" and the changes table says "Broaden body-based detection for non-array input" for this file. However, the only actual change in this file is the addition of a single clarifying comment on line 124. The detection logic itself is unchanged — it still returns "response" only when Array.isArray(requestBody.input).
The added comment (仅通过 input 数组识别；字符串/单对象简写由 response-input-rectifier 在端点确认后规范化) actually says the opposite of broadening: it explicitly documents that string/object inputs are not detected by the body-based fallback and are instead handled by the rectifier post-endpoint-confirmation.
This is fine from a correctness standpoint (and the rationale is sound — broadening the body-based fallback risks misclassifying /v1/embeddings and other endpoints that also use an input field). However, the PR description should be updated to accurately reflect that this was a documentation/clarification change rather than a detection broadening.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/format-mapper.ts
Line: 123-127
Comment:
**PR description contradicts the actual code change**
The PR summary states *"Broadened `detectClientFormat()` body-based fallback to recognize string and single-object `input` as Response API format"* and the changes table says *"Broaden body-based detection for non-array input"* for this file. However, the only actual change in this file is the addition of a single clarifying comment on line 124. The detection logic itself is **unchanged** — it still returns `"response"` only when `Array.isArray(requestBody.input)`.
The added comment (`仅通过 input 数组识别；字符串/单对象简写由 response-input-rectifier 在端点确认后规范化`) actually says the **opposite** of broadening: it explicitly documents that string/object inputs are *not* detected by the body-based fallback and are instead handled by the rectifier post-endpoint-confirmation.
This is fine from a correctness standpoint (and the rationale is sound — broadening the body-based fallback risks misclassifying `/v1/embeddings` and other endpoints that also use an `input` field). However, the PR description should be updated to accurately reflect that this was a documentation/clarification change rather than a detection broadening.
How can I resolve this? If you propose a fix, please make it concise.| // Case 3: 单对象 (MessageInput 有 role, ToolOutputsInput 有 type) | ||
| if (typeof input === "object" && input !== null) { | ||
| const obj = input as Record<string, unknown>; | ||
| if ("role" in obj || "type" in obj) { | ||
| message.input = [input]; | ||
| return { applied: true, action: "object_to_array", originalType: "object" }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Overly broad type key check may silently wrap unexpected objects
Case 3 wraps any object that contains either a "role" or "type" key. The "type" branch is intended to capture ToolOutputsInput items (e.g., { type: "function_call_output", ... }), but it will also match objects that happen to carry an unrelated "type" field — for example, a tool-definition object like { type: "bash_20250124", name: "bash" } or a content-block descriptor that a client accidentally placed at the top level of input.
When such an object is wrapped and forwarded, the downstream API will return an error referencing an unexpected array element rather than an obviously malformed top-level input field, making the failure harder to diagnose.
Consider tightening the guard to a known set of valid type values for ToolOutputsInput:
const TOOL_OUTPUT_TYPES = new Set([
"function_call_output",
"computer_call_output",
"web_search_call_output",
]);
if ("role" in obj || (typeof obj.type === "string" && TOOL_OUTPUT_TYPES.has(obj.type))) {
message.input = [input];
return { applied: true, action: "object_to_array", originalType: "object" };
}This keeps the role branch open (valid for MessageInput) while constraining the type branch to known tool-output shapes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/response-input-rectifier.ts
Line: 59-66
Comment:
**Overly broad `type` key check may silently wrap unexpected objects**
Case 3 wraps any object that contains either a `"role"` or `"type"` key. The `"type"` branch is intended to capture `ToolOutputsInput` items (e.g., `{ type: "function_call_output", ... }`), but it will also match objects that happen to carry an unrelated `"type"` field — for example, a tool-definition object like `{ type: "bash_20250124", name: "bash" }` or a content-block descriptor that a client accidentally placed at the top level of `input`.
When such an object is wrapped and forwarded, the downstream API will return an error referencing an unexpected array element rather than an obviously malformed top-level `input` field, making the failure harder to diagnose.
Consider tightening the guard to a known set of valid `type` values for `ToolOutputsInput`:
```typescript
const TOOL_OUTPUT_TYPES = new Set([
"function_call_output",
"computer_call_output",
"web_search_call_output",
]);
if ("role" in obj || (typeof obj.type === "string" && TOOL_OUTPUT_TYPES.has(obj.type))) {
message.input = [input];
return { applied: true, action: "object_to_array", originalType: "object" };
}
```
This keeps the `role` branch open (valid for `MessageInput`) while constraining the `type` branch to known tool-output shapes.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
messages/zh-CN/settings/config.json (1)
47-48:⚠️ Potential issue | 🟡 Minor说明文案把适用对象写宽了。
当前整流规则只会包装带
role的单对象；type不是触发条件。这里写成“带role/type的单消息对象”会让用户误以为仅带type的对象也会被规范化，建议收紧成“字符串简写或带role的单消息对象”。建议修改
- "enableResponseInputRectifierDesc": "自动将 /v1/responses 请求中的非数组 input（字符串简写或带 role/type 的单消息对象）规范化为标准数组格式后再处理（默认开启）。", + "enableResponseInputRectifierDesc": "自动将 /v1/responses 请求中的非数组 input（字符串简写或带 role 的单消息对象）规范化为标准数组格式后再处理（默认开启）。",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/zh-CN/settings/config.json` around lines 47 - 48, The description string for enableResponseInputRectifier is too broad: it currently says it normalizes "字符串简写或带 role/type 的单消息对象" but the rectifier only triggers for single message objects that have role, not type. Update the value for the key "enableResponseInputRectifierDesc" to say "字符串简写或带 role 的单消息对象" so it accurately reflects the implemented behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/en/settings/config.json`:
- Around line 58-59: The description string for the setting
enableResponseInputRectifier is too broad; update the value of
enableResponseInputRectifierDesc to clarify that only a "single message object
with a role" is normalized (not any single object). Locate the key
enableResponseInputRectifierDesc and edit its English text to replace "single
message object" with "single message object with a role" so the capability scope
is accurate and not overstated.
In `@messages/ja/settings/config.json`:
- Around line 58-59: The Japanese description for enableResponseInputRectifier
is overly broad; update the value of the enableResponseInputRectifierDesc entry
to state that only single message objects that include a role are normalized
(e.g., replace "role/type 付き単一メッセージオブジェクト" with "role を含む単一メッセージオブジェクト" or an
equivalent phrasing) so the text matches the actual behavior; change the string
associated with the enableResponseInputRectifierDesc key accordingly while
leaving enableResponseInputRectifier untouched.
---
Duplicate comments:
In `@messages/zh-CN/settings/config.json`:
- Around line 47-48: The description string for enableResponseInputRectifier is
too broad: it currently says it normalizes "字符串简写或带 role/type 的单消息对象" but the
rectifier only triggers for single message objects that have role, not type.
Update the value for the key "enableResponseInputRectifierDesc" to say "字符串简写或带
role 的单消息对象" so it accurately reflects the implemented behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c18358a-6bd3-4936-9d77-7ab545b6968a
📒 Files selected for processing (9)
messages/en/settings/config.jsonmessages/ja/settings/config.jsonmessages/ru/settings/config.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/settings/config.jsonsrc/app/v1/_lib/proxy/format-mapper.tssrc/app/v1/_lib/proxy/response-input-rectifier.tssrc/repository/system-config.tstests/unit/proxy/response-input-rectifier.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- messages/ru/settings/config.json
- tests/unit/proxy/response-input-rectifier.test.ts
- messages/zh-TW/settings/config.json
- src/app/v1/_lib/proxy/response-input-rectifier.ts
- src/app/v1/_lib/proxy/format-mapper.ts
| "enableResponseInputRectifier": "Enable Response Input Rectifier", | ||
| "enableResponseInputRectifierDesc": "Automatically normalizes non-array input (string shortcut or single message object) in /v1/responses requests to standard array format before processing (enabled by default).", |
There was a problem hiding this comment.
英文说明缺少 role 前提。
当前实现并不会规范化任意“single message object”，而是只处理带 role 的单对象。建议把文案收紧到 single message object with a role，避免把能力范围描述得过宽。
建议修改
- "enableResponseInputRectifierDesc": "Automatically normalizes non-array input (string shortcut or single message object) in /v1/responses requests to standard array format before processing (enabled by default).",
+ "enableResponseInputRectifierDesc": "Automatically normalizes non-array input (string shortcut or single message object with a role) in /v1/responses requests to standard array format before processing (enabled by default).",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/en/settings/config.json` around lines 58 - 59, The description
string for the setting enableResponseInputRectifier is too broad; update the
value of enableResponseInputRectifierDesc to clarify that only a "single message
object with a role" is normalized (not any single object). Locate the key
enableResponseInputRectifierDesc and edit its English text to replace "single
message object" with "single message object with a role" so the capability scope
is accurate and not overstated.
| "enableResponseInputRectifier": "Response Input 整流器を有効化", | ||
| "enableResponseInputRectifierDesc": "/v1/responses リクエストの非配列 input（文字列ショートカットまたは role/type 付き単一メッセージオブジェクト）を処理前に標準の配列形式に自動正規化します（既定で有効）。", |
There was a problem hiding this comment.
日文说明同样把适用范围写宽了。
这里不是 role/type 都会触发整流，而是只处理带 role 的单对象。建议把文案改成 role を含む単一メッセージオブジェクト（或等价表述），避免和实际行为不一致。
建议修改
- "enableResponseInputRectifierDesc": "/v1/responses リクエストの非配列 input（文字列ショートカットまたは role/type 付き単一メッセージオブジェクト）を処理前に標準の配列形式に自動正規化します（既定で有効）。",
+ "enableResponseInputRectifierDesc": "/v1/responses リクエストの非配列 input（文字列ショートカットまたは role を含む単一メッセージオブジェクト）を処理前に標準の配列形式に自動正規化します（既定で有効）。",📝 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.
| "enableResponseInputRectifier": "Response Input 整流器を有効化", | |
| "enableResponseInputRectifierDesc": "/v1/responses リクエストの非配列 input（文字列ショートカットまたは role/type 付き単一メッセージオブジェクト）を処理前に標準の配列形式に自動正規化します（既定で有効）。", | |
| "enableResponseInputRectifier": "Response Input 整流器を有効化", | |
| "enableResponseInputRectifierDesc": "/v1/responses リクエストの非配列 input（文字列ショートカットまたは role を含む単一メッセージオブジェクト）を処理前に標準の配列形式に自動正規化します（既定で有効）。", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/ja/settings/config.json` around lines 58 - 59, The Japanese
description for enableResponseInputRectifier is overly broad; update the value
of the enableResponseInputRectifierDesc entry to state that only single message
objects that include a role are normalized (e.g., replace "role/type
付き単一メッセージオブジェクト" with "role を含む単一メッセージオブジェクト" or an equivalent phrasing) so the
text matches the actual behavior; change the string associated with the
enableResponseInputRectifierDesc key accordingly while leaving
enableResponseInputRectifier untouched.
1 participant
Summary
/v1/responsesinputfield from string shortcut or single object format into the standard array format, before the guard pipeline processes the requestsession.addSpecialSetting()and persisted tomessage_requests.special_settingsJSONB columndetectClientFormat()body-based fallback to recognize string and single-objectinputas Response API formatRelated PRs:
Changes
response-input-rectifier.tsproxy-handler.tsformat-mapper.tsspecial-settings.ts,system-config.tsResponseInputRectifierSpecialSetting, settings toggleschema.ts,0079_easy_zeigeist.sqlenable_response_input_rectifiercolumn (default true)system-settings-cache.ts,system-config.ts,transformers.tsschemas.tssystem-settings-form.tsx,page.tsxsystem-config.tsmessages/*/settings/config.json(5 locales)response-input-rectifier.test.tsNormalization Rules
string(non-empty)string_to_array[{role:"user",content:[{type:"input_text",text}]}]""(empty)empty_string_to_empty_array[]{role:"user",...}object_to_array[input][...](array)passthroughundefined/nullpassthroughTest plan
bun run typecheckcleanbun run lintcleancurlwith string input to/v1/responsesDescription enhanced by Claude AI
Greptile Summary
This PR adds a
ResponseInputRectifierthat normalizes non-arrayinputfields on/v1/responsesrequests (string shorthand, single-object) into the standard array format before the guard pipeline runs, with full audit logging viasession.addSpecialSetting(), a DB-backed settings toggle (default enabled), UI controls, and i18n for five locales.The implementation is well-structured and follows the established rectifier pattern in the codebase. Key observations:
message_requests.special_settingsJSONB; passthrough cases (already-array input) are intentionally not audited, consistent with other rectifiers.true, matching every other rectifier in the system.format-mapper.tsas "Broaden body-based detection for non-array input", but the actual diff is a single added comment. The comment documents the opposite — that broadening was deliberately avoided to prevent misclassifying/v1/embeddingsrequests.typekey check in Case 3:rectifyResponseInputwraps any object containing a"type"key, which could silently wrap unrelated objects and produce harder-to-diagnose downstream API errors.Confidence Score: 4/5
"type" in objcheck in Case 3 that could silently wrap unrelated objects, and the misleading PR description for the format-mapper change.src/app/v1/_lib/proxy/response-input-rectifier.ts(Case 3typekey check) and the PR description forsrc/app/v1/_lib/proxy/format-mapper.ts.Important Files Changed
"type" in objcheck can silently wrap unrelated objects.rectifyResponseInputand 4 integration tests fornormalizeResponseInputcovering enabled/disabled paths and audit recording.ResponseInputRectifierSpecialSettingtype added and correctly included in theSpecialSettingunion. Type structure matches the rectifier's result shape.ALTER TABLEaddingenable_response_input_rectifier boolean DEFAULT true NOT NULL. Migration is minimal and backward compatible.DEFAULT_SETTINGSand the fail-open fallback object, defaulting totrueconsistent with the DB column default.Sequence Diagram
sequenceDiagram participant Client participant ProxyHandler participant FormatMapper participant ResponseInputRectifier participant SystemSettingsCache participant GuardPipeline participant Upstream Client->>ProxyHandler: POST /v1/responses (input: string | object | array) ProxyHandler->>FormatMapper: detectFormatByEndpoint("/v1/responses") FormatMapper-->>ProxyHandler: "response" ProxyHandler->>ProxyHandler: session.setOriginalFormat("response") ProxyHandler->>ResponseInputRectifier: normalizeResponseInput(session) ResponseInputRectifier->>SystemSettingsCache: getCachedSystemSettings() SystemSettingsCache-->>ResponseInputRectifier: { enableResponseInputRectifier: true } alt input is string ResponseInputRectifier->>ResponseInputRectifier: rectifyResponseInput → string_to_array ResponseInputRectifier->>ProxyHandler: session.addSpecialSetting(response_input_rectifier) else input is object with role/type ResponseInputRectifier->>ResponseInputRectifier: rectifyResponseInput → object_to_array ResponseInputRectifier->>ProxyHandler: session.addSpecialSetting(response_input_rectifier) else input is array or null/undefined ResponseInputRectifier->>ResponseInputRectifier: rectifyResponseInput → passthrough (no audit) end ResponseInputRectifier-->>ProxyHandler: void (message.input now array) ProxyHandler->>GuardPipeline: pipeline.run(session) [sees normalized input] GuardPipeline-->>ProxyHandler: null (pass) ProxyHandler->>Upstream: forward request Upstream-->>ProxyHandler: response ProxyHandler-->>Client: responseLast reviewed commit: 7c0880e