mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:40:44 +00:00
fix(github-copilot): preserve reasoning IDs for Copilot Codex models (#71684)
* fix(github-copilot): preserve all reasoning IDs and add gpt-5.3-codex support
The existing guard (8fd15ed0e5) only skipped rewriting reasoning item IDs
when encrypted_content was a non-null string. When gpt-5.3-codex is used
via GitHub Copilot, the model falls through to the forward-compat catch-all
with reasoning:false, so encrypted_content is never requested and arrives
as null — bypassing the guard and causing a rewrite. Copilot validates
reasoning item IDs server-side regardless of whether the client includes
encrypted_content, so the rewritten id triggers the 400 error.
Two changes:
1. connection-bound-ids.ts: skip ALL reasoning items unconditionally.
Reasoning items always reference server-side state bound to their
original ID; rewriting any of them breaks Copilot's lookup.
2. models.ts + index.ts: extend the forward-compat cloning logic to
cover gpt-5.3-codex (adds it to the template-target set and to
CODEX_TEMPLATE_MODEL_IDS so it can also serve as a template source
for gpt-5.4). Adds gpt-5.3-codex to COPILOT_XHIGH_MODEL_IDS for
the thinking profile.
Thanks @InvalidPandaa.
* docs(github-copilot): clarify gpt-5.3-codex is a no-op template for itself
https://claude.ai/code/session_01EAFmq4WyKkiUkVAqRXp4Bm
* fix(github-copilot): remove dead reasoning prefix branch in deriveReplacementId
https://claude.ai/code/session_01EAFmq4WyKkiUkVAqRXp4Bm
* fix(github-copilot): align reasoning id replay tests
* test(plugin-sdk): use cjs sidecar for require fast path
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -5,14 +5,14 @@ import {
|
||||
} from "./connection-bound-ids.js";
|
||||
|
||||
describe("github-copilot connection-bound response IDs", () => {
|
||||
it("rewrites opaque response item IDs deterministically", () => {
|
||||
const originalId = Buffer.from(`reasoning-${"x".repeat(24)}`).toString("base64");
|
||||
const first = [{ id: originalId, type: "reasoning" }];
|
||||
const second = [{ id: originalId, type: "reasoning" }];
|
||||
it("rewrites opaque message response item IDs deterministically", () => {
|
||||
const originalId = Buffer.from(`message-${"x".repeat(24)}`).toString("base64");
|
||||
const first = [{ id: originalId, type: "message" }];
|
||||
const second = [{ id: originalId, type: "message" }];
|
||||
|
||||
expect(rewriteCopilotConnectionBoundResponseIds(first)).toBe(true);
|
||||
expect(rewriteCopilotConnectionBoundResponseIds(second)).toBe(true);
|
||||
expect(first[0]?.id).toMatch(/^rs_[a-f0-9]{16}$/);
|
||||
expect(first[0]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
expect(first[0]?.id).toBe(second[0]?.id);
|
||||
});
|
||||
|
||||
@@ -35,26 +35,20 @@ describe("github-copilot connection-bound response IDs", () => {
|
||||
expect(input[4]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
});
|
||||
|
||||
it("preserves reasoning IDs when encrypted_content is present", () => {
|
||||
const originalId = Buffer.from(`reasoning-${"e".repeat(24)}`).toString("base64");
|
||||
it("preserves reasoning IDs regardless of encrypted_content", () => {
|
||||
const withEncrypted = Buffer.from(`reasoning-${"e".repeat(24)}`).toString("base64");
|
||||
const withNull = Buffer.from(`reasoning-${"n".repeat(24)}`).toString("base64");
|
||||
const withoutField = Buffer.from(`reasoning-${"a".repeat(24)}`).toString("base64");
|
||||
const input = [
|
||||
{
|
||||
id: originalId,
|
||||
type: "reasoning",
|
||||
encrypted_content: "opaque-encrypted-payload",
|
||||
},
|
||||
{ id: withEncrypted, type: "reasoning", encrypted_content: "opaque-encrypted-payload" },
|
||||
{ id: withNull, type: "reasoning", encrypted_content: null },
|
||||
{ id: withoutField, type: "reasoning" },
|
||||
];
|
||||
|
||||
expect(rewriteCopilotConnectionBoundResponseIds(input)).toBe(false);
|
||||
expect(input[0]?.id).toBe(originalId);
|
||||
});
|
||||
|
||||
it("still rewrites reasoning IDs when encrypted_content is absent", () => {
|
||||
const originalId = Buffer.from(`reasoning-${"n".repeat(24)}`).toString("base64");
|
||||
const input = [{ id: originalId, type: "reasoning" }];
|
||||
|
||||
expect(rewriteCopilotConnectionBoundResponseIds(input)).toBe(true);
|
||||
expect(input[0]?.id).toMatch(/^rs_[a-f0-9]{16}$/);
|
||||
expect(input[0]?.id).toBe(withEncrypted);
|
||||
expect(input[1]?.id).toBe(withNull);
|
||||
expect(input[2]?.id).toBe(withoutField);
|
||||
});
|
||||
|
||||
it("patches response payload input arrays only", () => {
|
||||
|
||||
@@ -18,7 +18,7 @@ function looksLikeConnectionBoundId(id: string): boolean {
|
||||
}
|
||||
|
||||
function deriveReplacementId(type: string | undefined, originalId: string): string {
|
||||
const prefix = type === "reasoning" ? "rs" : type === "function_call" ? "fc" : "msg";
|
||||
const prefix = type === "function_call" ? "fc" : "msg";
|
||||
const hex = createHash("sha256").update(originalId).digest("hex").slice(0, 16);
|
||||
return `${prefix}_${hex}`;
|
||||
}
|
||||
@@ -35,7 +35,11 @@ export function rewriteCopilotConnectionBoundResponseIds(input: unknown): boolea
|
||||
if (typeof id !== "string" || id.length === 0) {
|
||||
continue;
|
||||
}
|
||||
if (item.type === "reasoning" && typeof item.encrypted_content === "string") {
|
||||
// Reasoning items always reference server-side encrypted state bound to the
|
||||
// original item ID. Rewriting the ID — even when encrypted_content is absent
|
||||
// or null — breaks Copilot's server-side lookup and causes a 400 validation
|
||||
// failure regardless of whether the client included encrypted_content.
|
||||
if (item.type === "reasoning") {
|
||||
continue;
|
||||
}
|
||||
if (looksLikeConnectionBoundId(id)) {
|
||||
|
||||
@@ -12,7 +12,7 @@ import { buildGithubCopilotReplayPolicy } from "./replay-policy.js";
|
||||
import { wrapCopilotProviderStream } from "./stream.js";
|
||||
|
||||
const COPILOT_ENV_VARS = ["COPILOT_GITHUB_TOKEN", "GH_TOKEN", "GITHUB_TOKEN"];
|
||||
const COPILOT_XHIGH_MODEL_IDS = ["gpt-5.4", "gpt-5.2", "gpt-5.2-codex"] as const;
|
||||
const COPILOT_XHIGH_MODEL_IDS = ["gpt-5.4", "gpt-5.3-codex", "gpt-5.2", "gpt-5.2-codex"] as const;
|
||||
|
||||
type GithubCopilotPluginConfig = {
|
||||
discovery?: {
|
||||
|
||||
@@ -135,6 +135,50 @@ describe("resolveCopilotForwardCompatModel", () => {
|
||||
expect((result as unknown as Record<string, unknown>).reasoning).toBe(true);
|
||||
});
|
||||
|
||||
it("clones gpt-5.3-codex template for gpt-5.3-codex when not in registry", () => {
|
||||
const template = {
|
||||
id: "gpt-5.2-codex",
|
||||
name: "gpt-5.2-codex",
|
||||
provider: "github-copilot",
|
||||
api: "openai-responses",
|
||||
reasoning: true,
|
||||
contextWindow: 200_000,
|
||||
};
|
||||
const ctx = createMockCtx("gpt-5.3-codex", {
|
||||
"github-copilot/gpt-5.2-codex": template,
|
||||
});
|
||||
const result = requireResolvedModel(ctx);
|
||||
expect(result.id).toBe("gpt-5.3-codex");
|
||||
expect(result.name).toBe("gpt-5.3-codex");
|
||||
expect((result as unknown as Record<string, unknown>).reasoning).toBe(true);
|
||||
});
|
||||
|
||||
it("prefers gpt-5.3-codex as template source over gpt-5.2-codex for gpt-5.4", () => {
|
||||
const template53 = {
|
||||
id: "gpt-5.3-codex",
|
||||
name: "gpt-5.3-codex",
|
||||
provider: "github-copilot",
|
||||
api: "openai-responses",
|
||||
reasoning: true,
|
||||
contextWindow: 300_000,
|
||||
};
|
||||
const template52 = {
|
||||
id: "gpt-5.2-codex",
|
||||
name: "gpt-5.2-codex",
|
||||
provider: "github-copilot",
|
||||
api: "openai-responses",
|
||||
reasoning: true,
|
||||
contextWindow: 200_000,
|
||||
};
|
||||
const ctx = createMockCtx("gpt-5.4", {
|
||||
"github-copilot/gpt-5.3-codex": template53,
|
||||
"github-copilot/gpt-5.2-codex": template52,
|
||||
});
|
||||
const result = requireResolvedModel(ctx);
|
||||
expect(result.id).toBe("gpt-5.4");
|
||||
expect((result as unknown as Record<string, unknown>).contextWindow).toBe(300_000);
|
||||
});
|
||||
|
||||
it("falls through to synthetic catch-all when codex template is missing", () => {
|
||||
const ctx = createMockCtx("gpt-5.4");
|
||||
const result = requireResolvedModel(ctx);
|
||||
@@ -158,11 +202,20 @@ describe("resolveCopilotForwardCompatModel", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("infers reasoning=true for Codex model IDs", () => {
|
||||
for (const id of ["gpt-5.4-codex", "gpt-5.5-codex", "gpt-5.4-codex-mini", "gpt-5.3-codex"]) {
|
||||
const ctx = createMockCtx(id);
|
||||
const result = requireResolvedModel(ctx);
|
||||
expect((result as unknown as Record<string, unknown>).reasoning).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("sets reasoning=false for non-reasoning model IDs including mid-string o1/o3", () => {
|
||||
for (const id of [
|
||||
"gpt-5.4-mini",
|
||||
"claude-sonnet-4.6",
|
||||
"gpt-4o",
|
||||
"mycodexmodel",
|
||||
"audio-o1-hd",
|
||||
"turbo-o3-voice",
|
||||
]) {
|
||||
|
||||
@@ -6,12 +6,18 @@ import { normalizeModelCompat } from "openclaw/plugin-sdk/provider-model-shared"
|
||||
import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/text-runtime";
|
||||
|
||||
export const PROVIDER_ID = "github-copilot";
|
||||
const CODEX_GPT_54_MODEL_ID = "gpt-5.4";
|
||||
const CODEX_TEMPLATE_MODEL_IDS = ["gpt-5.2-codex"] as const;
|
||||
const CODEX_FORWARD_COMPAT_TARGET_IDS = new Set(["gpt-5.4", "gpt-5.3-codex"]);
|
||||
// gpt-5.3-codex is only a useful template when gpt-5.4 is the target; it is
|
||||
// always a registry miss (and therefore skipped) when it is the target itself.
|
||||
const CODEX_TEMPLATE_MODEL_IDS = ["gpt-5.3-codex", "gpt-5.2-codex"] as const;
|
||||
|
||||
const DEFAULT_CONTEXT_WINDOW = 128_000;
|
||||
const DEFAULT_MAX_TOKENS = 8192;
|
||||
|
||||
function isCopilotCodexModelId(modelId: string): boolean {
|
||||
return /(?:^|[-_.])codex(?:$|[-_.])/.test(modelId);
|
||||
}
|
||||
|
||||
export function resolveCopilotTransportApi(
|
||||
modelId: string,
|
||||
): "anthropic-messages" | "openai-responses" {
|
||||
@@ -35,9 +41,9 @@ export function resolveCopilotForwardCompatModel(
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// For gpt-5.4 specifically, clone from the gpt-5.2-codex template
|
||||
// to preserve any special settings the registry has for codex models.
|
||||
if (lowerModelId === CODEX_GPT_54_MODEL_ID) {
|
||||
// For gpt-5.4 and gpt-5.3-codex, clone from a registered codex template
|
||||
// to inherit the correct reasoning and capability flags.
|
||||
if (CODEX_FORWARD_COMPAT_TARGET_IDS.has(lowerModelId)) {
|
||||
for (const templateId of CODEX_TEMPLATE_MODEL_IDS) {
|
||||
const template = ctx.modelRegistry.find(
|
||||
PROVIDER_ID,
|
||||
@@ -60,7 +66,7 @@ export function resolveCopilotForwardCompatModel(
|
||||
// model isn't available on the user's plan. This lets new models be used
|
||||
// by simply adding them to agents.defaults.models in openclaw.json — no
|
||||
// code change required.
|
||||
const reasoning = /^o[13](\b|$)/.test(lowerModelId);
|
||||
const reasoning = /^o[13](\b|$)/.test(lowerModelId) || isCopilotCodexModelId(lowerModelId);
|
||||
return normalizeModelCompat({
|
||||
id: trimmedModelId,
|
||||
name: trimmedModelId,
|
||||
|
||||
@@ -101,11 +101,17 @@ describe("wrapCopilotAnthropicStream", () => {
|
||||
expect(baseStreamFn).toHaveBeenCalledWith(expect.anything(), expect.anything(), options);
|
||||
});
|
||||
|
||||
it("adds Copilot headers and rewrites Responses connection-bound IDs before payload send", () => {
|
||||
const connectionBoundId = Buffer.from(`reasoning-${"x".repeat(24)}`).toString("base64");
|
||||
it("adds Copilot headers, preserves reasoning IDs, and rewrites message IDs before payload send", () => {
|
||||
const reasoningId = Buffer.from(`reasoning-${"x".repeat(24)}`).toString("base64");
|
||||
const messageId = Buffer.from(`message-${"y".repeat(24)}`).toString("base64");
|
||||
const payloads: Array<{ input: Array<Record<string, unknown>> }> = [];
|
||||
const baseStreamFn = vi.fn((_model, _context, options) => {
|
||||
const payload = { input: [{ id: connectionBoundId, type: "reasoning" }] };
|
||||
const payload = {
|
||||
input: [
|
||||
{ id: reasoningId, type: "reasoning" },
|
||||
{ id: messageId, type: "message" },
|
||||
],
|
||||
};
|
||||
options?.onPayload?.(payload, _model);
|
||||
payloads.push(payload);
|
||||
return {
|
||||
@@ -144,7 +150,8 @@ describe("wrapCopilotAnthropicStream", () => {
|
||||
"X-Test": "1",
|
||||
},
|
||||
});
|
||||
expect(payloads[0]?.input[0]?.id).toMatch(/^rs_[a-f0-9]{16}$/);
|
||||
expect(payloads[0]?.input[0]?.id).toBe(reasoningId);
|
||||
expect(payloads[0]?.input[1]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
});
|
||||
|
||||
it("rewrites Copilot Responses IDs returned by an existing payload hook", async () => {
|
||||
|
||||
Reference in New Issue
Block a user