mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-26 12:26:38 -06:00
fix(github-copilot): preserve reasoning during tool calls (#112659)
* fix(github-copilot): preserve active encrypted reasoning Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(github-copilot): normalize null reasoning status Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test(github-copilot): cover adjacent reasoning cleanup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -22,7 +22,7 @@ describe("github-copilot connection-bound response IDs", () => {
|
||||
const functionCallId = Buffer.from(`function-call-${"y".repeat(20)}`).toString("base64");
|
||||
const messageId = Buffer.from(`message-${"z".repeat(24)}`).toString("base64");
|
||||
const input = [
|
||||
{ id: "rs_existing", type: "reasoning" },
|
||||
{ id: "rs_existing", type: "reasoning", encrypted_content: "active" },
|
||||
{ id: "msg_existing", type: "message" },
|
||||
{ id: "fc_existing", type: "function_call" },
|
||||
{ id: functionCallId, type: "function_call" },
|
||||
@@ -37,53 +37,42 @@ describe("github-copilot connection-bound response IDs", () => {
|
||||
expect(input[4]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
});
|
||||
|
||||
it("preserves valid reasoning IDs but strips 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");
|
||||
it("preserves complete active reasoning and omits connection-bound IDs", () => {
|
||||
const connectionBoundId = Buffer.from(`reasoning-${"e".repeat(24)}`).toString("base64");
|
||||
const input = [
|
||||
{ id: withEncrypted, type: "reasoning", encrypted_content: "opaque-encrypted-payload" },
|
||||
{ id: withNull, type: "reasoning", encrypted_content: null },
|
||||
{ id: withoutField, type: "reasoning" },
|
||||
];
|
||||
|
||||
expect(rewriteInputIds(input)).toBe(true);
|
||||
expect(input[0]).toEqual({ id: withEncrypted, type: "reasoning" });
|
||||
expect(input[1]).toEqual({ id: withNull, type: "reasoning" });
|
||||
expect(input[2]).toEqual({ id: withoutField, type: "reasoning" });
|
||||
});
|
||||
|
||||
it("strips encrypted_content from valid reasoning IDs at payload send time", () => {
|
||||
const withEncrypted = "abcDEF0123+/=";
|
||||
const withoutEncrypted = "reasoning/abc+123=";
|
||||
const input = [
|
||||
{ id: withEncrypted, type: "reasoning", encrypted_content: "opaque-encrypted-payload" },
|
||||
{ id: withoutEncrypted, type: "reasoning" },
|
||||
];
|
||||
|
||||
expect(rewriteInputIds(input)).toBe(true);
|
||||
expect(input[0]).toEqual({ id: withEncrypted, type: "reasoning" });
|
||||
expect(input[1]).toEqual({ id: withoutEncrypted, type: "reasoning" });
|
||||
});
|
||||
|
||||
it("drops unsafe reasoning IDs and strips encrypted_content from kept items", () => {
|
||||
const overlongId = `5PX6gLHXT5wE+Y2tPmUV4gn+${"B".repeat(384)}`;
|
||||
const input = [
|
||||
{
|
||||
id: overlongId,
|
||||
type: "reasoning",
|
||||
encrypted_content: "encrypted-replay-payload",
|
||||
summary: [],
|
||||
},
|
||||
{ type: "reasoning", encrypted_content: "missing-id", summary: [] },
|
||||
{ id: 123, type: "reasoning", encrypted_content: "non-string-id", summary: [] },
|
||||
{ id: "rs_valid", type: "reasoning", encrypted_content: "valid", summary: [] },
|
||||
{ id: "rs_active", type: "reasoning", encrypted_content: "native", summary: [] },
|
||||
{ type: "reasoning", status: "completed", encrypted_content: "idless" },
|
||||
{ id: connectionBoundId, type: "reasoning", status: null, encrypted_content: "connection" },
|
||||
];
|
||||
|
||||
expect(rewriteInputIds(input)).toBe(true);
|
||||
expect(input).toEqual([
|
||||
{ type: "reasoning", summary: [] },
|
||||
{ id: "rs_valid", type: "reasoning", summary: [] },
|
||||
{ id: "rs_active", type: "reasoning", encrypted_content: "native", summary: [] },
|
||||
{ type: "reasoning", status: "completed", encrypted_content: "idless" },
|
||||
{ type: "reasoning", encrypted_content: "connection" },
|
||||
]);
|
||||
expect(rewriteInputIds(input)).toBe(false);
|
||||
});
|
||||
|
||||
it("drops adjacent incomplete or foreign reasoning and only clears dependent assistant IDs", () => {
|
||||
const invalidReasoning = [
|
||||
{ id: "thinking_0", type: "reasoning", encrypted_content: "foreign" },
|
||||
{ id: `rs_${"r".repeat(64)}`, type: "reasoning", encrypted_content: "oversized" },
|
||||
{ id: "rs_missing", type: "reasoning" },
|
||||
{ id: "rs_empty", type: "reasoning", encrypted_content: "" },
|
||||
{ id: "rs_i", type: "reasoning", status: "incomplete", encrypted_content: "partial" },
|
||||
{ id: 123, type: "reasoning", encrypted_content: "malformed" },
|
||||
];
|
||||
const input: Array<Record<string, unknown>> = [
|
||||
...invalidReasoning,
|
||||
{ id: "msg_signed", type: "message", role: "assistant" },
|
||||
{ id: "msg_user", type: "message", role: "user" },
|
||||
];
|
||||
|
||||
expect(rewriteInputIds(input)).toBe(true);
|
||||
expect(input).toEqual([
|
||||
{ type: "message", role: "assistant" },
|
||||
{ id: "msg_user", type: "message", role: "user" },
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
@@ -31,7 +31,20 @@ function isInputItem(value: unknown): value is InputItem {
|
||||
}
|
||||
|
||||
function isValidReasoningReplayId(id: unknown): id is string {
|
||||
return typeof id === "string" && id.length > 0 && id.length <= 64;
|
||||
return typeof id === "string" && id.length <= 64 && /^rs_[A-Za-z0-9_-]+$/.test(id);
|
||||
}
|
||||
|
||||
function dropReasoningItem(input: unknown[], index: number): void {
|
||||
input.splice(index, 1);
|
||||
const dependentMessage = input[index];
|
||||
// Assistant replay IDs are signed with preceding reasoning; keeping one after a drop is invalid.
|
||||
if (
|
||||
isInputItem(dependentMessage) &&
|
||||
dependentMessage.type === "message" &&
|
||||
dependentMessage.role === "assistant"
|
||||
) {
|
||||
delete dependentMessage.id;
|
||||
}
|
||||
}
|
||||
|
||||
function sanitizeCopilotReplayResponseIds(input: unknown): boolean {
|
||||
@@ -39,21 +52,33 @@ function sanitizeCopilotReplayResponseIds(input: unknown): boolean {
|
||||
return false;
|
||||
}
|
||||
let rewrote = false;
|
||||
// Walk backward because dropping reasoning splices input and must not skip adjacent items.
|
||||
for (let index = input.length - 1; index >= 0; index -= 1) {
|
||||
const item = input[index];
|
||||
if (!isInputItem(item)) {
|
||||
continue;
|
||||
}
|
||||
const id = item.id;
|
||||
// Reasoning encrypted_content is tied to the Copilot connection token,
|
||||
// which rotates per request. Drop items with unsafe IDs; strip
|
||||
// encrypted_content from kept items so summary-only replay is sent.
|
||||
if (item.type === "reasoning") {
|
||||
if (id !== undefined && !isValidReasoningReplayId(id)) {
|
||||
input.splice(index, 1);
|
||||
// Cold reasoning is removed earlier; normalize null status and never synthesize active IDs.
|
||||
if (item.status === null) {
|
||||
delete item.status;
|
||||
rewrote = true;
|
||||
} else if ("encrypted_content" in item) {
|
||||
delete item.encrypted_content;
|
||||
}
|
||||
const isComplete =
|
||||
typeof item.encrypted_content === "string" &&
|
||||
item.encrypted_content.length > 0 &&
|
||||
(item.status === undefined || item.status === "completed");
|
||||
if (!isComplete) {
|
||||
dropReasoningItem(input, index);
|
||||
rewrote = true;
|
||||
} else if (id === undefined || isValidReasoningReplayId(id)) {
|
||||
continue;
|
||||
} else if (typeof id === "string" && looksLikeConnectionBoundId(id)) {
|
||||
delete item.id;
|
||||
rewrote = true;
|
||||
} else {
|
||||
dropReasoningItem(input, index);
|
||||
rewrote = true;
|
||||
}
|
||||
continue;
|
||||
|
||||
@@ -332,7 +332,7 @@ describe("github-copilot plugin", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("owns Claude replay thinking cleanup", () => {
|
||||
it("owns session-bound replay thinking cleanup", () => {
|
||||
const provider = registerProviderWithPluginConfig({});
|
||||
const messages = [
|
||||
{
|
||||
@@ -368,8 +368,20 @@ describe("github-copilot plugin", () => {
|
||||
]);
|
||||
expect(
|
||||
provider.sanitizeReplayHistory?.({
|
||||
modelId: "gpt-5.4",
|
||||
modelApi: "openai-responses",
|
||||
modelId: "gpt-5.4",
|
||||
messages,
|
||||
} as never),
|
||||
).toEqual([
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "visible" }],
|
||||
},
|
||||
]);
|
||||
expect(
|
||||
provider.sanitizeReplayHistory?.({
|
||||
modelApi: "openai-completions",
|
||||
modelId: "gpt-5.4",
|
||||
messages,
|
||||
} as never),
|
||||
).toBe(messages);
|
||||
|
||||
@@ -87,7 +87,7 @@ describe("sanitizeGithubCopilotReplayHistory", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("passes history through on OpenAI-compatible transports", () => {
|
||||
it("strips replayed thinking on the OpenAI Responses transport", () => {
|
||||
expect(
|
||||
sanitizeGithubCopilotReplayHistory({
|
||||
provider: "github-copilot",
|
||||
@@ -95,6 +95,17 @@ describe("sanitizeGithubCopilotReplayHistory", () => {
|
||||
modelId: "gpt-5.4",
|
||||
messages,
|
||||
} as never),
|
||||
).toEqual([{ role: "assistant", content: [{ type: "text", text: "visible" }] }]);
|
||||
});
|
||||
|
||||
it("passes history through on the OpenAI Completions transport", () => {
|
||||
expect(
|
||||
sanitizeGithubCopilotReplayHistory({
|
||||
provider: "github-copilot",
|
||||
modelApi: "openai-completions",
|
||||
modelId: "gpt-5.4",
|
||||
messages,
|
||||
} as never),
|
||||
).toBe(messages);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -66,7 +66,7 @@ export function buildGithubCopilotReplayPolicy(
|
||||
}
|
||||
|
||||
export function sanitizeGithubCopilotReplayHistory(ctx: ProviderSanitizeReplayHistoryContext) {
|
||||
return isCopilotAnthropicTransport(ctx.modelApi)
|
||||
return ctx.modelApi === "openai-responses" || isCopilotAnthropicTransport(ctx.modelApi)
|
||||
? stripCopilotAssistantThinkingMessages(ctx.messages)
|
||||
: ctx.messages;
|
||||
}
|
||||
|
||||
@@ -8,6 +8,8 @@ import { describe, expect, it, vi } from "vitest";
|
||||
import { COPILOT_RUNTIME_INTEGRATION_ID } from "./runtime-identity.js";
|
||||
import { wrapCopilotAnthropicStream, wrapCopilotProviderStream } from "./stream.js";
|
||||
|
||||
type ResponsesTestPayload = { input: Array<Record<string, unknown>> };
|
||||
|
||||
function requireStreamFn(streamFn: ReturnType<typeof wrapCopilotProviderStream>) {
|
||||
expect(streamFn).toBeTypeOf("function");
|
||||
if (!streamFn) {
|
||||
@@ -591,20 +593,21 @@ describe("wrapCopilotAnthropicStream", () => {
|
||||
|
||||
it("adds Copilot headers, sanitizes reasoning replay, and rewrites message IDs before payload send", () => {
|
||||
const reasoningId = Buffer.from(`reasoning-${"x".repeat(24)}`).toString("base64");
|
||||
const overlongReasoningId = `5PX6gLHXT5wE+Y2tPmUV4gn+${"B".repeat(384)}`;
|
||||
const messageId = Buffer.from(`message-${"y".repeat(24)}`).toString("base64");
|
||||
const payloads: Array<{ input: Array<Record<string, unknown>> }> = [];
|
||||
const payloads: ResponsesTestPayload[] = [];
|
||||
const baseStreamFn = vi.fn((_model, _context, options) => {
|
||||
const payload = {
|
||||
input: [
|
||||
{ id: "rs_active", type: "reasoning", encrypted_content: "native-encrypted" },
|
||||
{ type: "reasoning", status: null, encrypted_content: "idless-encrypted", summary: [] },
|
||||
{ id: reasoningId, type: "reasoning", encrypted_content: "valid-encrypted-payload" },
|
||||
{ type: "reasoning", encrypted_content: "idless-encrypted-payload", summary: [] },
|
||||
{
|
||||
id: overlongReasoningId,
|
||||
id: "thinking_0",
|
||||
type: "reasoning",
|
||||
encrypted_content: "invalid-encrypted-payload",
|
||||
summary: [],
|
||||
},
|
||||
{ id: "msg_signed", type: "message", role: "assistant" },
|
||||
{ id: messageId, type: "message" },
|
||||
],
|
||||
};
|
||||
@@ -649,45 +652,63 @@ describe("wrapCopilotAnthropicStream", () => {
|
||||
},
|
||||
onPayload: options.onPayload,
|
||||
});
|
||||
expect(payloads[0]?.input[0]?.id).toBe(reasoningId);
|
||||
expect(payloads[0]?.input[0]?.id).toBe("rs_active");
|
||||
expect(payloads[0]?.input.map((item) => item.type)).toEqual([
|
||||
"reasoning",
|
||||
"reasoning",
|
||||
"reasoning",
|
||||
"message",
|
||||
"message",
|
||||
]);
|
||||
expect(payloads[0]?.input[1]?.id).toBeUndefined();
|
||||
expect(payloads[0]?.input[2]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
expect(payloads[0]?.input[0]).not.toHaveProperty("encrypted_content");
|
||||
expect(payloads[0]?.input[1]).not.toHaveProperty("encrypted_content");
|
||||
expect(payloads[0]?.input[2]?.id).toBeUndefined();
|
||||
expect(payloads[0]?.input[3]?.id).toBeUndefined();
|
||||
expect(payloads[0]?.input[4]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
expect(payloads[0]?.input.slice(0, 3).every((item) => item.encrypted_content)).toBe(true);
|
||||
});
|
||||
|
||||
it("rewrites Copilot Responses IDs returned by an existing payload hook", async () => {
|
||||
const connectionBoundId = Buffer.from(`message-${"y".repeat(24)}`).toString("base64");
|
||||
let returnedPayload: unknown;
|
||||
const baseStreamFn = vi.fn(async (_model, _context, options) => {
|
||||
returnedPayload = await options?.onPayload?.({ input: [] }, _model);
|
||||
return {
|
||||
async *[Symbol.asyncIterator]() {},
|
||||
} as never;
|
||||
});
|
||||
it("sanitizes all sync and async Copilot Responses hook outcomes", async () => {
|
||||
const model = {
|
||||
provider: "github-copilot",
|
||||
api: "openai-responses",
|
||||
id: "gpt-5.4",
|
||||
} as never;
|
||||
const context = { messages: [{ role: "user", content: "hi" }] } as never;
|
||||
|
||||
const wrapped = requireStreamFn(wrapCopilotProviderStream({ streamFn: baseStreamFn } as never));
|
||||
for (const asyncHook of [false, true]) {
|
||||
for (const replace of [false, true]) {
|
||||
const connectionBoundId = Buffer.from(`message-${"y".repeat(24)}`).toString("base64");
|
||||
let originalPayload: ResponsesTestPayload = { input: [] };
|
||||
let hookResult: unknown;
|
||||
const baseStreamFn = vi.fn((_model, _context, options) => {
|
||||
originalPayload = { input: [] };
|
||||
hookResult = options?.onPayload?.(originalPayload, _model);
|
||||
return {
|
||||
async *[Symbol.asyncIterator]() {},
|
||||
} as never;
|
||||
});
|
||||
|
||||
await wrapped(
|
||||
{
|
||||
provider: "github-copilot",
|
||||
api: "openai-responses",
|
||||
id: "gpt-5.4",
|
||||
} as never,
|
||||
{ messages: [{ role: "user", content: "hi" }] } as never,
|
||||
{
|
||||
onPayload: () => ({ input: [{ id: connectionBoundId, type: "message" }] }),
|
||||
} as never,
|
||||
);
|
||||
const wrapped = requireStreamFn(
|
||||
wrapCopilotProviderStream({ streamFn: baseStreamFn } as never),
|
||||
);
|
||||
const mutatePayload = (payload: ResponsesTestPayload) => {
|
||||
const target: ResponsesTestPayload = replace ? { input: [] } : payload;
|
||||
target.input.push({ id: connectionBoundId, type: "message" });
|
||||
return replace ? target : undefined;
|
||||
};
|
||||
const onPayload = asyncHook
|
||||
? async (payload: ResponsesTestPayload) => mutatePayload(payload)
|
||||
: (payload: ResponsesTestPayload) => mutatePayload(payload);
|
||||
|
||||
expect((returnedPayload as { input: Array<Record<string, unknown>> }).input[0]?.id).toMatch(
|
||||
/^msg_[a-f0-9]{16}$/,
|
||||
);
|
||||
void wrapped(model, context, { onPayload } as never);
|
||||
|
||||
expect(hookResult instanceof Promise).toBe(asyncHook);
|
||||
const replacement = await hookResult;
|
||||
expect(replacement !== undefined).toBe(replace);
|
||||
const returnedPayload = (replacement ?? originalPayload) as ResponsesTestPayload;
|
||||
expect(returnedPayload.input[0]?.id).toMatch(/^msg_[a-f0-9]{16}$/);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("adds Copilot headers for Chat Completions models", () => {
|
||||
|
||||
@@ -236,7 +236,7 @@ function wrapCopilotOpenAIResponsesStream(
|
||||
headers: buildCopilotRequestHeaders(context, options?.headers),
|
||||
onPayload: (payload, payloadModel) => {
|
||||
sanitizeCopilotReplayResponsePayload(payload);
|
||||
return patchOnPayloadResult(originalOnPayload?.(payload, payloadModel));
|
||||
return patchOnPayloadResult(originalOnPayload?.(payload, payloadModel), undefined, payload);
|
||||
},
|
||||
};
|
||||
return underlying(model, context, wrappedOptions);
|
||||
|
||||
Reference in New Issue
Block a user