Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 134 additions & 24 deletions packages/core/src/core/openaiContentGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,14 @@ export class OpenAIContentGenerator implements ContentGenerator {
/**
* Reinitialize the OpenAI client with current environment variables
*/
public updateClient(): void {
updateClient(): void {
this.initializeClient();
}

/**
* Update the model being used
*/
public updateModel(model: string): void {
updateModel(model: string): void {
this.model = model;
console.log('[DEBUG] Updated model to:', this.model);
}
Comment on lines +143 to 153
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify impact of visibility change.

Removing the explicit public modifiers changes these methods to package-private visibility. If external code calls updateClient() or updateModel(), this is a breaking change.

Run the following script to check for external usage:


🏁 Script executed:

#!/bin/bash
# Search for calls to updateClient and updateModel outside this file
rg -n --type=ts -C3 '\b(updateClient|updateModel)\s*\(' \
  -g '!packages/core/src/core/openaiContentGenerator.ts'

Length of output: 1562


Revert visibility changes or document breaking change.

External code calls updateModel() at packages/cli/src/ui/hooks/slashCommandProcessor.ts:839 and updateClient() at packages/cli/src/ui/hooks/slashCommandProcessor.ts:896. Removing the public modifiers breaks these call sites. Either restore the public modifiers or update the external callers.

🤖 Prompt for AI Agents
In packages/core/src/core/openaiContentGenerator.ts around lines 143 to 153, the
methods' visibility was changed causing external callers in
packages/cli/src/ui/hooks/slashCommandProcessor.ts (lines ~839 and ~896) to
break; restore the original public visibility on updateClient() and
updateModel() (i.e., re-add the public modifiers) so existing external call
sites continue to work, or if you prefer to keep them non-public, update the two
external callers to use a supported public API instead and run the typecheck to
confirm no other call sites are affected.

Expand Down Expand Up @@ -1154,7 +1154,9 @@ export class OpenAIContentGenerator implements ContentGenerator {
args = JSON.parse(toolCall.function.arguments);
} catch (error) {
console.error('Failed to parse function arguments:', error);
args = {};
console.error('Problematic arguments:', toolCall.function.arguments);
// Try to extract partial JSON or provide a fallback
args = this.extractPartialJson(toolCall.function.arguments) || {};
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Redact the tool-call payloads in error logs.

Both error paths now print the entire arguments string when JSON parsing fails. Those blobs routinely contain user data, so this change dumps PII straight into logs. Please log only metadata (e.g., message id/length) and apply the same redaction in the helper.

-              console.error('Problematic arguments:', toolCall.function.arguments);
+              console.error(
+                'Problematic tool arguments (length=%d)',
+                toolCall.function.arguments.length
+              );-                console.error('Problematic accumulated arguments:', accumulatedCall.arguments);
+                console.error(
+                  'Problematic accumulated tool arguments (length=%d)',
+                  accumulatedCall.arguments.length
+                );

Also redact the payload in extractPartialJson before returning the warning so we never emit the raw JSON.

Also applies to: 1274-1281

🤖 Prompt for AI Agents
In packages/core/src/core/openaiContentGenerator.ts around lines 1154-1160 (and
similarly 1274-1281), the code logs the full tool-call arguments when JSON.parse
fails; change those console.error calls to emit only safe metadata (e.g.,
message id, arguments length, and a short hash or truncated prefix) rather than
the raw payload, and replace the two raw-argument logs with a single metadata
log. Update the extractPartialJson helper so it never logs or returns the raw
JSON: when it cannot fully recover JSON, return a sanitized fallback (empty
object or redacted placeholder) and emit a warning that contains only metadata
(message id/length/placeholder) — do not include the original string. Ensure
both places use the same redaction utility function to produce consistent
metadata-only logs.

}

Expand Down Expand Up @@ -1273,6 +1275,9 @@ export class OpenAIContentGenerator implements ContentGenerator {
'Failed to parse final tool call arguments:',
error,
);
console.error('Problematic accumulated arguments:', accumulatedCall.arguments);
// Try to extract partial JSON or provide a fallback
args = this.extractPartialJson(accumulatedCall.arguments) || {};
}
}

Expand Down Expand Up @@ -1402,6 +1407,9 @@ export class OpenAIContentGenerator implements ContentGenerator {
return params;
}

/**
* Map OpenAI finish reasons to Gemini finish reasons
*/
private mapFinishReason(openaiReason: string | null): FinishReason {
if (!openaiReason) return FinishReason.FINISH_REASON_UNSPECIFIED;
const mapping: Record<string, FinishReason> = {
Expand All @@ -1414,6 +1422,33 @@ export class OpenAIContentGenerator implements ContentGenerator {
return mapping[openaiReason] || FinishReason.FINISH_REASON_UNSPECIFIED;
}

/**
* Map Gemini finish reasons to OpenAI finish reasons
*/
private mapGeminiFinishReasonToOpenAI(geminiReason?: unknown): string {
if (!geminiReason) return 'stop';

switch (geminiReason) {
case 'STOP':
case 1: // FinishReason.STOP
return 'stop';
case 'MAX_TOKENS':
case 2: // FinishReason.MAX_TOKENS
return 'length';
case 'SAFETY':
case 3: // FinishReason.SAFETY
return 'content_filter';
case 'RECITATION':
case 4: // FinishReason.RECITATION
return 'content_filter';
case 'OTHER':
case 5: // FinishReason.OTHER
return 'stop';
default:
return 'stop';
}
}

/**
* Convert Gemini request format to OpenAI chat completion format for logging
*/
Expand Down Expand Up @@ -1790,29 +1825,104 @@ export class OpenAIContentGenerator implements ContentGenerator {
}

/**
* Map Gemini finish reasons to OpenAI finish reasons
* Extract valid JSON from potentially partial JSON string
* This handles cases where streaming chunks contain incomplete JSON
*/
private mapGeminiFinishReasonToOpenAI(geminiReason?: unknown): string {
if (!geminiReason) return 'stop';
private extractPartialJson(input: string): Record<string, unknown> | null {
if (!input || typeof input !== 'string') {
return null;
}

switch (geminiReason) {
case 'STOP':
case 1: // FinishReason.STOP
return 'stop';
case 'MAX_TOKENS':
case 2: // FinishReason.MAX_TOKENS
return 'length';
case 'SAFETY':
case 3: // FinishReason.SAFETY
return 'content_filter';
case 'RECITATION':
case 4: // FinishReason.RECITATION
return 'content_filter';
case 'OTHER':
case 5: // FinishReason.OTHER
return 'stop';
default:
return 'stop';
// First try to parse the entire string
try {
return JSON.parse(input);
} catch {
// If that fails, try to find valid JSON patterns
}

// Try to find a complete JSON object in the string
// This handles cases like: {"key": "value_partial
// or: {"key": "value"}
const trimmed = input.trim();

// Check if it looks like a complete object
if (trimmed.startsWith('{') && trimmed.endsWith('}')) {
// Try to parse character by character to find where it breaks
let braceCount = 0;
let inString = false;
let escapeNext = false;

for (let i = 0; i < trimmed.length; i++) {
const char = trimmed[i];

if (escapeNext) {
escapeNext = false;
continue;
}

if (char === '\\') {
escapeNext = true;
continue;
}

if (char === '"' && !escapeNext) {
inString = !inString;
continue;
}

if (!inString) {
if (char === '{') braceCount++;
else if (char === '}') braceCount--;

// If we have balanced braces and are at the end, try parsing
if (braceCount === 0 && i === trimmed.length - 1) {
try {
return JSON.parse(trimmed);
} catch {
// Still not valid, continue
}
}
}
}
}

// Try to extract key-value pairs manually for simple cases
// This handles: key1="value1",key2="value2"
const keyValuePattern = /"([^"]+)"\s*:\s*("([^"]*)"|([0-9.]+)|(true|false)|(null))/g;
const matches = [...trimmed.matchAll(keyValuePattern)];

if (matches.length > 0) {
const result: Record<string, unknown> = {};

for (const match of matches) {
const key = match[1];
let value: unknown;

if (match[3] !== undefined) {
// String value
value = match[3];
} else if (match[4] !== undefined) {
// Number value
value = parseFloat(match[4]);
if (Number.isNaN(value)) {
value = match[4];
}
} else if (match[5] !== undefined) {
// Boolean value
value = match[5] === 'true';
} else if (match[6] !== undefined) {
// Null value
value = null;
}

result[key] = value;
}

return result;
}

// Last resort: return an empty object rather than throwing
console.warn('Could not extract valid JSON from:', input);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Apply the same redaction inside extractPartialJson.

This helper still logs the full input string on failure, undoing the redaction above. Please switch to a metadata-only message (length, hash, etc.) so we don’t leak request content.

-    console.warn('Could not extract valid JSON from:', input);
+    console.warn(
+      'Could not extract valid JSON from tool arguments (length=%d)',
+      input.length
+    );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Last resort: return an empty object rather than throwing
console.warn('Could not extract valid JSON from:', input);
// Last resort: return an empty object rather than throwing
console.warn(
'Could not extract valid JSON from tool arguments (length=%d)',
input.length
);
🤖 Prompt for AI Agents
In packages/core/src/core/openaiContentGenerator.ts around lines 1924-1925, the
extractPartialJson helper currently logs the full input on failure; change that
to avoid leaking request content by replacing the console.warn call with a
metadata-only log: compute and log the input length and a short cryptographic
hash (e.g., first 8-12 chars of a SHA-256 hex) plus a clear message and any
parse error details, but do NOT include the raw input string itself; ensure
dependencies used for hashing are already imported or use Node's crypto module.

return null;
}
}
Loading