Close out the last review edge-cases before final merge
The remaining review threads were down to route alias hygiene, proxy-safe header ordering, and a Gemini canonical tool-result mismatch. This commit makes the alias/websocket behavior explicit, preserves contributor helper safety, and teaches the canonical Gemini path to recover tool names and wrap function results the same way the OpenAI-shaped path already does. Constraint: Keep the last review fixes limited to the current branch so the release PR stays easy to reason about Rejected: Defer the Gemini canonical fix to a later refactor | the review found a real functional mismatch and the fix stayed localized to the request bridge Confidence: high Scope-risk: narrow Directive: Keep canonical-to-provider bridges internally consistent; if one path resolves tool names or wraps results, the sibling path should not diverge silently Tested: Focused vitest for Gemini request bridge, route aliases, upstream request builder, responses response bridge, contributor updater; typecheck; git diff --check Not-tested: Full npm test; any broader end-to-end Gemini live-provider behavior outside the covered bridge tests
This commit is contained in:
@@ -552,8 +552,8 @@ async function handleResponsesWebsocketConnection(
|
|||||||
request: IncomingMessage,
|
request: IncomingMessage,
|
||||||
authContext: ResponsesWebsocketAuthContext,
|
authContext: ResponsesWebsocketAuthContext,
|
||||||
) {
|
) {
|
||||||
const websocketSessionId = headerValueToTrimmedString(request.headers['session_id'])
|
const websocketSessionId = headerValueToTrimmedString(request.headers['session-id'])
|
||||||
|| headerValueToTrimmedString(request.headers['session-id'])
|
|| headerValueToTrimmedString(request.headers['session_id'])
|
||||||
|| headerValueToTrimmedString(request.headers['conversation-id'])
|
|| headerValueToTrimmedString(request.headers['conversation-id'])
|
||||||
|| headerValueToTrimmedString(request.headers['conversation_id'])
|
|| headerValueToTrimmedString(request.headers['conversation_id'])
|
||||||
|| randomUUID();
|
|| randomUUID();
|
||||||
|
|||||||
@@ -262,4 +262,57 @@ describe('gemini generate-content request bridge', () => {
|
|||||||
expect(functionCallCount).toBe(2);
|
expect(functionCallCount).toBe(2);
|
||||||
expect(functionResponseCount).toBe(2);
|
expect(functionResponseCount).toBe(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('uses canonical tool-call names for Gemini functionResponse parts', () => {
|
||||||
|
const body = buildCanonicalRequestToGeminiGenerateContentBody({
|
||||||
|
operation: 'generate',
|
||||||
|
surface: 'gemini-generate-content',
|
||||||
|
cliProfile: 'generic',
|
||||||
|
requestedModel: 'gemini-2.5-pro',
|
||||||
|
stream: false,
|
||||||
|
messages: [
|
||||||
|
{
|
||||||
|
role: 'assistant',
|
||||||
|
parts: [{
|
||||||
|
type: 'tool_call',
|
||||||
|
id: 'call_weather',
|
||||||
|
name: 'lookup_weather',
|
||||||
|
argumentsJson: '{"city":"Paris"}',
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: 'tool',
|
||||||
|
parts: [{
|
||||||
|
type: 'tool_result',
|
||||||
|
toolCallId: 'call_weather',
|
||||||
|
resultJson: { temperature: '22C' },
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(body.contents).toEqual([
|
||||||
|
{
|
||||||
|
role: 'model',
|
||||||
|
parts: [{
|
||||||
|
functionCall: {
|
||||||
|
id: 'call_weather',
|
||||||
|
name: 'lookup_weather',
|
||||||
|
args: { city: 'Paris' },
|
||||||
|
},
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
role: 'user',
|
||||||
|
parts: [{
|
||||||
|
functionResponse: {
|
||||||
|
name: 'lookup_weather',
|
||||||
|
response: {
|
||||||
|
result: { temperature: '22C' },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -328,7 +328,10 @@ export function buildGeminiGenerateContentRequestFromOpenAi(input: {
|
|||||||
return request;
|
return request;
|
||||||
}
|
}
|
||||||
|
|
||||||
function canonicalPartToGeminiPart(part: CanonicalContentPart): Record<string, unknown> | null {
|
function canonicalPartToGeminiPart(
|
||||||
|
part: CanonicalContentPart,
|
||||||
|
toolNameById?: ReadonlyMap<string, string>,
|
||||||
|
): Record<string, unknown> | null {
|
||||||
if (part.type === 'text') {
|
if (part.type === 'text') {
|
||||||
return {
|
return {
|
||||||
text: part.text,
|
text: part.text,
|
||||||
@@ -391,10 +394,13 @@ function canonicalPartToGeminiPart(part: CanonicalContentPart): Record<string, u
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (part.type === 'tool_result') {
|
if (part.type === 'tool_result') {
|
||||||
|
const response = part.resultJson ?? parseJsonString(part.resultText ?? '');
|
||||||
return {
|
return {
|
||||||
functionResponse: {
|
functionResponse: {
|
||||||
name: part.toolCallId,
|
name: toolNameById?.get(part.toolCallId) || 'unknown',
|
||||||
response: part.resultJson ?? parseJsonString(part.resultText ?? ''),
|
response: {
|
||||||
|
result: response,
|
||||||
|
},
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -407,19 +413,26 @@ export function buildCanonicalRequestToGeminiGenerateContentBody(
|
|||||||
): Record<string, unknown> {
|
): Record<string, unknown> {
|
||||||
const contents: Array<Record<string, unknown>> = [];
|
const contents: Array<Record<string, unknown>> = [];
|
||||||
const systemParts: Array<Record<string, unknown>> = [];
|
const systemParts: Array<Record<string, unknown>> = [];
|
||||||
|
const toolNameById = new Map<string, string>();
|
||||||
|
|
||||||
for (const message of request.messages) {
|
for (const message of request.messages) {
|
||||||
if (message.role === 'system' || message.role === 'developer') {
|
if (message.role === 'system' || message.role === 'developer') {
|
||||||
systemParts.push(
|
systemParts.push(
|
||||||
...message.parts
|
...message.parts
|
||||||
.map((part) => canonicalPartToGeminiPart(part))
|
.map((part) => canonicalPartToGeminiPart(part, toolNameById))
|
||||||
.filter((part): part is Record<string, unknown> => !!part),
|
.filter((part): part is Record<string, unknown> => !!part),
|
||||||
);
|
);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
for (const part of message.parts) {
|
||||||
|
if (part.type === 'tool_call') {
|
||||||
|
toolNameById.set(part.id, part.name);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
const parts = message.parts
|
const parts = message.parts
|
||||||
.map((part) => canonicalPartToGeminiPart(part))
|
.map((part) => canonicalPartToGeminiPart(part, toolNameById))
|
||||||
.filter((part): part is Record<string, unknown> => !!part);
|
.filter((part): part is Record<string, unknown> => !!part);
|
||||||
|
|
||||||
if (parts.length <= 0) continue;
|
if (parts.length <= 0) continue;
|
||||||
|
|||||||
Reference in New Issue
Block a user