Repository navigation
fix(codex): sanitize raw responses input #1895
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type JsonRecord = Record<string, unknown>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function toRecord(value: unknown): JsonRecord | null { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return value && typeof value === "object" && !Array.isArray(value) ? (value as JsonRecord) : null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function isResponsesMessageItem(record: JsonRecord): boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return record.type === "message" || (!record.type && typeof record.role === "string"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function isInternalAssistantMessage(record: JsonRecord): boolean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!isResponsesMessageItem(record)) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (record.role !== "assistant") return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const phase = typeof record.phase === "string" ? record.phase.trim().toLowerCase() : ""; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!phase) return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // OpenCode can send assistant-side commentary/analysis frames in Responses | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // shape. Those frames are local runtime state, not durable conversation turns. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return phase !== "final"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function sanitizeResponsesInputItems(items: readonly unknown[]): unknown[] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sanitized: unknown[] = []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const item of items) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const record = toRecord(item); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (record && isInternalAssistantMessage(record)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sanitized.push(structuredClone(item)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return sanitized; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gemini-code-assist fixed in eddc3d6.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the update and for confirming the validation steps. The implementation in |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the
bodyobject was already deep-cloned at the beginning oftransformRequest(line 1165), performing another deep clone of each item inbody.inputinsidesanitizeResponsesInputItemsis redundant. If you apply the suggested change tosanitizeResponsesInputItemsto make cloning optional, you can call it here withfalseto improve performance.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gemini-code-assist fixed in eddc3d6. The Codex path now calls
sanitizeResponsesInputItems(body.input, false)to avoid a second deep clone aftertransformRequesthas already cloned the whole request body.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the update, @dhaern. That change correctly addresses the redundancy while maintaining the necessary sanitization logic.