-
Notifications
You must be signed in to change notification settings - Fork 657
fix(providers): normalize GitHub Copilot streamed Responses wire #996
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 all commits
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 |
|---|---|---|
|
|
@@ -169,7 +169,11 @@ import { | |
| imageGenToolCallAliases, | ||
| restoreImageGenCallsInJson, | ||
| } from "../responses-image-gen-repair"; | ||
| import { composeSsePayloadRewrites, relaySseWithPayloadRewrite } from "../sse-payload-rewrite"; | ||
| import { | ||
| composeSsePayloadRewrites, | ||
| createGithubCopilotObfuscationRewrite, | ||
| relaySseWithPayloadRewrite, | ||
| } from "../sse-payload-rewrite"; | ||
| import type { EffectiveSubagentRoster, SpawnAgentSurface } from "../../codex/catalog"; | ||
|
|
||
| import { buildToolBridgeMaps, collabSurface, injectDeveloperMessage, multiAgentGuidanceText } from "./collaboration"; | ||
|
|
@@ -1972,6 +1976,9 @@ async function handleResponsesInner( | |
| hasResponsesItemIdRepair(repairConfig) | ||
| ? createResponsesItemIdPayloadRewrite(repairConfig!, translatorBudget) | ||
| : undefined, | ||
| route.providerName === "github-copilot" | ||
| ? createGithubCopilotObfuscationRewrite() | ||
| : undefined, | ||
|
Comment on lines
+1979
to
+1981
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. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Include the Copilot rewrite in For Copilot-only traffic, Add a dedicated Copilot boolean to Proposed fix const repairConfig = route.provider.responsesItemIdRepair;
- const needsClientRewrite = imageGenCallAliases.size > 0 || hasResponsesItemIdRepair(repairConfig);
+ const needsGithubCopilotRewrite = route.providerName === "github-copilot";
+ const needsClientRewrite = imageGenCallAliases.size > 0
+ || hasResponsesItemIdRepair(repairConfig)
+ || needsGithubCopilotRewrite;
const payloadRewrites = [
createImageGenCallRestoreRewrite(imageGenCallAliases),
hasResponsesItemIdRepair(repairConfig)
? createResponsesItemIdPayloadRewrite(repairConfig!, translatorBudget)
: undefined,
- route.providerName === "github-copilot"
+ needsGithubCopilotRewrite
? createGithubCopilotObfuscationRewrite()
: undefined,
].filter((rewrite): rewrite is NonNullable<typeof rewrite> => rewrite !== undefined);As per path instructions, “A behavior change in src/ should come with a focused regression test near existing tests for that subsystem.” 🤖 Prompt for AI AgentsSource: Path instructions |
||
| ].filter((rewrite): rewrite is NonNullable<typeof rewrite> => rewrite !== undefined); | ||
| // #864: win32 rewrite traffic must never enter the tee()+JS-pull chain | ||
| // (Bun#32111 JS-sink segfault — text frames pass, the terminal block is | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,26 @@ import type { TranslatorBudget } from "../lib/translator-budget"; | |
|
|
||
| export type SsePayloadRewrite = (payload: string) => string; | ||
|
|
||
| function isPayloadRecord(v: unknown): v is Record<string, unknown> { | ||
| return typeof v === "object" && v !== null && !Array.isArray(v); | ||
| } | ||
|
|
||
| /** | ||
| * GitHub Copilot encrypts reasoning summaries (and re-encrypts them per event) with its own | ||
| * scheme, which Responses clients cannot decrypt; the app then stays stuck on "thinking". | ||
| * Replace the ciphertext with a canonical empty plaintext summary so the item completes | ||
| * immediately and the turn moves on to the actual output. | ||
| */ | ||
| function sanitizeGithubCopilotReasoningItem(item: Record<string, unknown>): boolean { | ||
| if (item.type !== "reasoning" || typeof item.encrypted_content !== "string") return false; | ||
| delete item.encrypted_content; | ||
| const summary = Array.isArray(item.summary) ? item.summary : []; | ||
| summary.push({ type: "summary_text", text: "" }); | ||
| item.summary = summary; | ||
| if (!Array.isArray(item.content)) item.content = []; | ||
| return true; | ||
| } | ||
|
|
||
| /** Split one complete SSE event block while retaining its original blank-line delimiter. */ | ||
| export function nextSseBlock(buffer: string): { block: string; delimiter: string; rest: string } | null { | ||
| const match = buffer.match(/\r?\n\r?\n/); | ||
|
|
@@ -61,6 +81,108 @@ export function composeSsePayloadRewrites(...rewrites: SsePayloadRewrite[]): Sse | |
| }; | ||
| } | ||
|
|
||
| /** | ||
| * GitHub Copilot's `/responses` wire obfuscates streamed function-call argument | ||
| * deltas (ciphertext `delta` plus an `obfuscation` field) for the `vscode-chat` | ||
| * integration. Responses clients (codex-rs) cannot de-obfuscate them, so an | ||
| * agentic turn stalls on the first tool call. The terminal | ||
| * `response.function_call_arguments.done` event carries the plaintext | ||
| * `arguments`, so this rewrite drops the ciphertext deltas and re-emits the | ||
| * full plaintext arguments as a single delta immediately before `done`, keeping | ||
| * the stream protocol-conformant (deltas still concatenate to the final | ||
| * arguments). The `obfuscation` metadata is stripped from every event. | ||
| */ | ||
| export function createGithubCopilotObfuscationRewrite(): SsePayloadRewrite { | ||
| // GitHub re-encrypts `item_id` per event (delta and done carry different ids for the | ||
| // same logical call), so calls are tracked by the stable `output_index` instead. | ||
| const obfuscatedCalls = new Set<number>(); | ||
|
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.
If a buggy or hostile Copilot stream emits obfuscated deltas with many distinct Useful? React with 👍 / 👎. |
||
| // GitHub also re-encrypts the response id and every item id per event (`created`/`done` | ||
| // and the terminal `completed` payload disagree). Clients reconcile streamed items with | ||
| // the completed response by id, so pin every entity to its first-seen id. | ||
| let responseId: string | null = null; | ||
| const itemIds = new Map<number, string>(); | ||
| return (payload: string): string => { | ||
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(payload); | ||
| } catch { | ||
| return payload; | ||
| } | ||
| if (!isPayloadRecord(parsed)) return payload; | ||
| let changed = false; | ||
| if (typeof parsed.obfuscation === "string") { | ||
| delete parsed.obfuscation; | ||
| changed = true; | ||
| } | ||
| const type = parsed.type; | ||
| if (type === "response.function_call_arguments.delta" && changed) { | ||
| if (typeof parsed.output_index === "number") obfuscatedCalls.add(parsed.output_index); | ||
| // Ciphertext chunk: emit an empty delta so the client sees a valid event | ||
| // without any unusable bytes; the plaintext arrives with `.done`. | ||
| parsed.delta = ""; | ||
| return JSON.stringify(parsed); | ||
| } | ||
| if (type === "response.function_call_arguments.done" && typeof parsed.output_index === "number") { | ||
| if (obfuscatedCalls.delete(parsed.output_index)) { | ||
| const deltaEvent = { | ||
| type: "response.function_call_arguments.delta", | ||
| item_id: parsed.item_id, | ||
|
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.
When Copilot re-encrypts the function-call item's ID between AGENTS.md reference: src/AGENTS.md:L17-L19 Useful? React with 👍 / 👎. |
||
| output_index: parsed.output_index, | ||
| sequence_number: parsed.sequence_number, | ||
| delta: typeof parsed.arguments === "string" ? parsed.arguments : "", | ||
| }; | ||
| // The relay writes `data: <payload>` then the block delimiter, so this | ||
| // payload becomes two consecutive valid SSE events (delta, then done). | ||
| return `${JSON.stringify(deltaEvent)}\n\ndata: ${JSON.stringify(parsed)}`; | ||
|
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.
When Copilot sends standard named SSE blocks such as AGENTS.md reference: src/AGENTS.md:L17-L19 Useful? React with 👍 / 👎.
Comment on lines
+118
to
+136
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. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Verify whether optional generic item-ID repair currently covers GitHub Copilot.
rg -n -C 4 'responsesItemIdRepair|github-copilot' src
rg -n -C 4 'cipher-delta-id|cipher-done-id|item_id' tests/sse-payload-rewrite.test.tsRepository: lidge-jun/opencodex Length of output: 16633 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- outline ---'
ast-grep outline src/server/sse-payload-rewrite.ts
echo '--- implementation ---'
sed -n '1,190p' src/server/sse-payload-rewrite.ts
echo '--- relevant tests ---'
sed -n '1,120p' tests/sse-payload-rewrite.test.ts
echo '--- rewrite composition and config ---'
sed -n '1960,1985p' src/server/responses/core.ts
rg -n -C 8 'function hasResponsesItemIdRepair|createResponsesItemIdPayloadRewrite|createGithubCopilotObfuscationRewrite|itemIds' src/serverRepository: lidge-jun/opencodex Length of output: 27880 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- relay framing ---'
sed -n '180,280p' src/server/sse-payload-rewrite.ts
echo '--- remaining rewrite tests ---'
sed -n '120,260p' tests/sse-payload-rewrite.test.ts
echo '--- item-id repair state model ---'
sed -n '1,220p' src/server/responses-item-id-repair.tsRepository: lidge-jun/opencodex Length of output: 17836 🌐 Web query:
💡 Result: In the OpenAI Responses and Realtime APIs, streaming function call arguments is handled via specific server-sent events that allow developers to build arguments incrementally [1][2]. The two primary events for this process are: 1. response.function_call_arguments.delta: This event is emitted incrementally as the model generates the JSON arguments for a function [1][3]. It contains the delta string that should be appended to your local buffer for that specific function call [1][3]. Key fields include item_id (identifying the function call), output_index, and the delta string itself [1][3][4]. 2. response.function_call_arguments.done: This event is emitted once the model has finished generating all arguments for a function call [1][3]. It provides the finalized, complete argument string [1]. It also contains the item_id and output_index, allowing you to map the finished arguments back to the specific function call initiated [1][3]. Key technical details for implementation: - Buffering: You should maintain a buffer (usually a string or dictionary) for each active function call, keyed by its item_id or call_id [5][3]. - Incremental Updates: When a response.function_call_arguments.delta event arrives, append the delta field to the corresponding buffer [1][5][3]. Do not parse the JSON until the process is complete [3]. - Completion: Upon receiving response.function_call_arguments.done, you can safely parse the fully assembled JSON string to invoke your function [1][3]. - Item Mapping: Both events provide the item_id and output_index, which are critical for tracking multiple simultaneous function calls or managing state in complex responses [1][3][4]. Note that in some earlier versions of the SDK, developers reported issues where the function name field was missing in the done event [6]; ensuring you are using the latest stable version of the OpenAI SDK is recommended to avoid such issues [6]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
events = [
{
"type": "response.output_item.added",
"output_index": 0,
"item": {"type": "function_call", "id": "first-item-id"},
},
{
"type": "response.function_call_arguments.delta",
"item_id": "cipher-delta-id",
"output_index": 0,
"sequence_number": 3,
"delta": "ciphertext-chunk-1",
"obfuscation": "abc123",
},
{
"type": "response.function_call_arguments.done",
"item_id": "cipher-done-id",
"output_index": 0,
"sequence_number": 5,
"arguments": '{"a":2,"b":2}',
},
]
# Faithful behavioral probe of the inspected implementation's relevant branches.
obfuscated = set()
item_ids = {}
out = []
for event in events:
parsed = json.loads(json.dumps(event))
changed = False
if isinstance(parsed.get("obfuscation"), str):
del parsed["obfuscation"]
changed = True
typ = parsed.get("type")
if typ == "response.function_call_arguments.delta" and changed:
obfuscated.add(parsed["output_index"])
parsed["delta"] = ""
out.append(parsed)
continue
if typ == "response.function_call_arguments.done" and isinstance(parsed.get("output_index"), (int, float)):
if parsed["output_index"] in obfuscated:
obfuscated.remove(parsed["output_index"])
out.append({
"type": "response.function_call_arguments.delta",
"item_id": parsed.get("item_id"),
"output_index": parsed["output_index"],
"sequence_number": parsed.get("sequence_number"),
"delta": parsed.get("arguments", ""),
})
if typ in ("response.output_item.added", "response.output_item.done"):
item = parsed.get("item")
index = parsed.get("output_index")
if isinstance(index, (int, float)) and isinstance(item, dict) and isinstance(item.get("id"), str):
item_ids.setdefault(index, item["id"])
out.append(parsed)
print("remembered nested item id:", item_ids[0])
print("argument event item ids:", [e.get("item_id") for e in out if "item_id" in e])
print("synthetic delta item id:", out[2]["item_id"])
assert item_ids[0] == "first-item-id"
assert [e.get("item_id") for e in out if "item_id" in e] == [
"cipher-delta-id", "cipher-done-id", "cipher-done-id"
]
assert out[2]["item_id"] != item_ids[0]
print("result: top-level function-call item_id values are not pinned")
PYRepository: lidge-jun/opencodex Length of output: 381 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
events = [
{
"type": "response.output_item.added",
"output_index": 0,
"item": {"type": "function_call", "id": "first-item-id"},
},
{
"type": "response.function_call_arguments.delta",
"item_id": "cipher-delta-id",
"output_index": 0,
"sequence_number": 3,
"delta": "ciphertext-chunk-1",
"obfuscation": "abc123",
},
{
"type": "response.function_call_arguments.done",
"item_id": "cipher-done-id",
"output_index": 0,
"sequence_number": 5,
"arguments": '{"a":2,"b":2}',
},
]
obfuscated = set()
item_ids = {}
out = []
for event in events:
parsed = json.loads(json.dumps(event))
changed = False
if isinstance(parsed.get("obfuscation"), str):
del parsed["obfuscation"]
changed = True
typ = parsed.get("type")
if typ == "response.function_call_arguments.delta" and changed:
obfuscated.add(parsed["output_index"])
parsed["delta"] = ""
out.append(parsed)
continue
if typ == "response.function_call_arguments.done":
if parsed["output_index"] in obfuscated:
obfuscated.remove(parsed["output_index"])
out.append({
"type": "response.function_call_arguments.delta",
"item_id": parsed.get("item_id"),
"output_index": parsed["output_index"],
"sequence_number": parsed.get("sequence_number"),
"delta": parsed.get("arguments", ""),
})
if typ in ("response.output_item.added", "response.output_item.done"):
item = parsed.get("item")
index = parsed.get("output_index")
if isinstance(index, (int, float)) and isinstance(item, dict) and isinstance(item.get("id"), str):
item_ids.setdefault(index, item["id"])
out.append(parsed)
print("remembered nested item id:", item_ids[0])
print("argument event item ids:", [e.get("item_id") for e in out if "item_id" in e])
print("synthetic delta item id:", out[2]["item_id"])
assert item_ids[0] == "first-item-id"
assert [e.get("item_id") for e in out if "item_id" in e] == [
"cipher-delta-id", "cipher-done-id", "cipher-done-id"
]
assert out[2]["item_id"] != item_ids[0]
print("result: top-level function-call item_id values are not pinned")
PYRepository: lidge-jun/opencodex Length of output: 381 Normalize top-level
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
| if (type === "response.created" && isPayloadRecord(parsed.response)) { | ||
| const id = parsed.response.id; | ||
| if (typeof id === "string") { | ||
| if (responseId === null) responseId = id; | ||
| else if (id !== responseId) { | ||
| parsed.response.id = responseId; | ||
| changed = true; | ||
| } | ||
| } | ||
| } | ||
| if (type === "response.output_item.added" || type === "response.output_item.done") { | ||
| const outputIndex = parsed.output_index; | ||
| const item = parsed.item; | ||
| if (typeof outputIndex === "number" && isPayloadRecord(item) && typeof item.id === "string") { | ||
| const existing = itemIds.get(outputIndex); | ||
| if (existing === undefined) itemIds.set(outputIndex, item.id); | ||
| else if (item.id !== existing) { | ||
| item.id = existing; | ||
| changed = true; | ||
| } | ||
| } | ||
| if (isPayloadRecord(item) && sanitizeGithubCopilotReasoningItem(item)) changed = true; | ||
| } | ||
| if (type === "response.completed" && isPayloadRecord(parsed.response)) { | ||
| if (typeof parsed.response.id === "string" && responseId !== null && parsed.response.id !== responseId) { | ||
| parsed.response.id = responseId; | ||
|
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.
When Copilot gives Useful? React with 👍 / 👎. |
||
| changed = true; | ||
| } | ||
| if (Array.isArray(parsed.response.output)) { | ||
| parsed.response.output.forEach((item, outputIndex) => { | ||
| if (!isPayloadRecord(item)) return; | ||
| if (typeof item.id === "string") { | ||
| const existing = itemIds.get(outputIndex); | ||
| if (existing === undefined) itemIds.set(outputIndex, item.id); | ||
| else if (item.id !== existing) { | ||
| item.id = existing; | ||
| changed = true; | ||
| } | ||
| } | ||
| if (sanitizeGithubCopilotReasoningItem(item)) changed = true; | ||
| }); | ||
| } | ||
| } | ||
| return changed ? JSON.stringify(parsed) : payload; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Relay an SSE body through a single JS pull wrapper, rewriting each event's data payload in place. | ||
| * Non-data fields and framing are preserved; invalid JSON payloads are left to the rewrite callback. | ||
|
|
||
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.
In
src/server/responses/core.ts, adding the Copilot transform only topayloadRewritesleavesneedsClientRewritefalse when it is the sole rewrite. Consequently, macOS withstreamMode: "eager-relay"and Windows when eager relay is selected enter the branch at line 1992, butrewritePayloadis omitted becausewin32EagerRewriteis also false, so obfuscated Copilot events reach the client unchanged and tool turns still stall. Derive the rewrite requirement frompayloadRewrites.lengthor explicitly include Copilot so every eager path either applies the transform inline or remains on the rewriting tee path.AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.