Skip to content

Commit 08725b3

Browse files
authored
fix: normalize null copilot tool call types
1 parent 013496e commit 08725b3

3 files changed

Lines changed: 153 additions & 3 deletions

File tree

containers/api-proxy/providers/copilot.js

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* accepts OAuth tokens, not API keys.
1515
*/
1616

17-
const { normalizeApiTarget, normalizeBasePath, createAdapterMethods } = require('../proxy-utils');
17+
const { normalizeApiTarget, normalizeBasePath, createAdapterMethods, composeBodyTransforms } = require('../proxy-utils');
1818
const { URL } = require('url');
1919

2020
/**
@@ -137,6 +137,51 @@ function deriveGitHubApiBasePath(env = process.env) {
137137
}
138138
}
139139

140+
/**
141+
* Normalize OpenAI-style tool calls that omit the required type field.
142+
*
143+
* Some Copilot/OpenAI-compatible responses can echo tool_calls entries with a
144+
* null/undefined `type`, which later causes upstream validation failures when
145+
* the same message history is sent back. This transform patches only
146+
* function-style tool calls by setting `type: "function"`.
147+
*
148+
* @param {Buffer} body - Raw request body
149+
* @returns {Buffer|null} Updated body or null when no changes are needed
150+
*/
151+
function normalizeNullTypeToolCalls(body) {
152+
let parsed;
153+
try {
154+
parsed = JSON.parse(body.toString('utf8'));
155+
} catch {
156+
return null;
157+
}
158+
159+
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed) || !Array.isArray(parsed.messages)) {
160+
return null;
161+
}
162+
163+
let changed = false;
164+
for (const message of parsed.messages) {
165+
if (!message || typeof message !== 'object' || Array.isArray(message) || !Array.isArray(message.tool_calls)) {
166+
continue;
167+
}
168+
169+
for (const toolCall of message.tool_calls) {
170+
if (!toolCall || typeof toolCall !== 'object' || Array.isArray(toolCall)) continue;
171+
const hasFunctionPayload =
172+
toolCall.function &&
173+
typeof toolCall.function === 'object' &&
174+
!Array.isArray(toolCall.function);
175+
if (toolCall.type == null && hasFunctionPayload) {
176+
toolCall.type = 'function';
177+
changed = true;
178+
}
179+
}
180+
}
181+
182+
return changed ? Buffer.from(JSON.stringify(parsed)) : null;
183+
}
184+
140185
/**
141186
* Create the GitHub Copilot provider adapter.
142187
*
@@ -152,7 +197,10 @@ function createCopilotAdapter(env, deps = {}) {
152197
const rawTarget = deriveCopilotApiTarget(env);
153198
const basePath = normalizeBasePath(env.COPILOT_API_BASE_PATH);
154199

155-
const bodyTransform = deps.bodyTransform || null;
200+
const bodyTransform = composeBodyTransforms(
201+
deps.bodyTransform || null,
202+
normalizeNullTypeToolCalls
203+
);
156204

157205
// Pre-computed models path used by getModelsFetchConfig and getReflectionInfo.
158206
// For BYOK/custom providers the base path prefix is included (e.g. /api/v1/models
@@ -334,5 +382,6 @@ module.exports = {
334382
deriveCopilotApiTarget,
335383
deriveGitHubApiTarget,
336384
deriveGitHubApiBasePath,
385+
normalizeNullTypeToolCalls,
337386
},
338387
};

containers/api-proxy/server.auth.test.js

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
*/
66

77
const { shouldStripHeader } = require('./proxy-utils');
8-
const { _testing: { resolveCopilotAuthToken, stripBearerPrefix }, createCopilotAdapter } = require('./providers/copilot');
8+
const {
9+
_testing: { resolveCopilotAuthToken, stripBearerPrefix, normalizeNullTypeToolCalls },
10+
createCopilotAdapter,
11+
} = require('./providers/copilot');
912

1013
describe('shouldStripHeader', () => {
1114
it('should strip authorization header', () => {
@@ -135,6 +138,50 @@ describe('resolveCopilotAuthToken', () => {
135138
});
136139
});
137140

141+
describe('normalizeNullTypeToolCalls', () => {
142+
it('normalizes null tool_call type to "function" in outgoing message history', () => {
143+
const input = Buffer.from(JSON.stringify({
144+
model: 'gpt-5.4',
145+
messages: [
146+
{
147+
role: 'assistant',
148+
tool_calls: [
149+
{
150+
id: 'call_1',
151+
type: null,
152+
function: { name: 'edit', arguments: '{"path":"a.txt"}' },
153+
},
154+
],
155+
},
156+
],
157+
}));
158+
159+
const transformed = normalizeNullTypeToolCalls(input);
160+
expect(transformed).not.toBeNull();
161+
const parsed = JSON.parse(transformed.toString('utf8'));
162+
expect(parsed.messages[0].tool_calls[0].type).toBe('function');
163+
});
164+
165+
it('returns null when no tool_call type normalization is needed', () => {
166+
const input = Buffer.from(JSON.stringify({
167+
messages: [
168+
{
169+
role: 'assistant',
170+
tool_calls: [
171+
{
172+
id: 'call_1',
173+
type: 'function',
174+
function: { name: 'edit', arguments: '{}' },
175+
},
176+
],
177+
},
178+
],
179+
}));
180+
181+
expect(normalizeNullTypeToolCalls(input)).toBeNull();
182+
});
183+
});
184+
138185
// ── createCopilotAdapter — BYOK auth header format ───────────────────────────
139186
//
140187
// These tests guard against the "badly formatted Authorization header" bug in

containers/api-proxy/server.lifecycle.test.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,60 @@ describe('createProviderServer', () => {
416416
expect(authErrLog.message).toContain('400');
417417
});
418418

419+
it('normalizes null tool_call type before forwarding when upstream returns 400', async () => {
420+
const { lines, spy } = collectLogOutput();
421+
let writtenBody = null;
422+
jest.spyOn(https, 'request').mockImplementation((options, callback) => {
423+
const proxyReq = new EventEmitter();
424+
proxyReq.write = jest.fn((chunk) => {
425+
writtenBody = Buffer.isBuffer(chunk) ? chunk.toString('utf8') : String(chunk);
426+
});
427+
proxyReq.end = jest.fn(() => {
428+
setImmediate(() => {
429+
const proxyRes = new EventEmitter();
430+
proxyRes.statusCode = 400;
431+
proxyRes.headers = { 'content-type': 'application/json' };
432+
proxyRes.pipe = jest.fn((destRes) => { destRes.end('{}'); });
433+
callback(proxyRes);
434+
setImmediate(() => proxyRes.emit('end'));
435+
});
436+
});
437+
proxyReq.destroy = jest.fn();
438+
return proxyReq;
439+
});
440+
441+
const adapter = createCopilotAdapter({ COPILOT_API_KEY: 'sk-test' });
442+
const port = await startAdapter(adapter);
443+
const body = {
444+
model: 'gpt-5.4',
445+
messages: [
446+
{
447+
role: 'assistant',
448+
tool_calls: [
449+
{
450+
id: 'call_1',
451+
type: null,
452+
function: { name: 'edit', arguments: '{"path":"README.md"}' },
453+
},
454+
],
455+
},
456+
],
457+
};
458+
await fetch(port, '/v1/chat/completions', {
459+
method: 'POST',
460+
body: JSON.stringify(body),
461+
headers: { 'content-type': 'application/json' },
462+
});
463+
464+
jest.restoreAllMocks();
465+
spy.mockRestore();
466+
467+
const forwarded = JSON.parse(writtenBody);
468+
expect(forwarded.messages[0].tool_calls[0].type).toBe('function');
469+
const authErrLog = lines.find(l => l.event === 'upstream_auth_error' && l.status === 400);
470+
expect(authErrLog).toBeDefined();
471+
});
472+
419473
it('emits upstream_auth_error when upstream returns 401', async () => {
420474
const { lines, spy } = collectLogOutput();
421475
mockHttpsWithStatus(401);

0 commit comments

Comments
 (0)