fix(cliproxy): address PR #399 review suggestions

- Move require('os') to top-level import
- Add debug logging for log dir creation failures
- Add hash collision detection and warning in ToolNameMapper
- Clarify proxy chain order comments in cliproxy-executor
This commit is contained in:
kaitranntt
2026-01-29 17:49:32 -05:00
parent 149dfc3d6d
commit 98b7f6f454
3 changed files with 63 additions and 3 deletions
+3 -1
View File
@@ -844,7 +844,9 @@ export async function execClaudeWithCLIProxy(
// Tool sanitization proxy - applies to ALL CLIProxy providers.
// Sanitizes MCP tool names exceeding Gemini's 64-char limit.
// Chain: Claude CLI → ToolSanitizationProxy → [CodexReasoningProxy] → CLIProxy → Gemini
// Proxy chain order (request flow):
// Claude CLI → ToolSanitizationProxy → [CodexReasoningProxy if codex] → CLIProxy → Backend
// Response flow is reversed, with each proxy transforming data back.
let toolSanitizationProxy: ToolSanitizationProxy | null = null;
let toolSanitizationPort: number | null = null;
+42
View File
@@ -39,6 +39,12 @@ export interface SanitizationChange {
sanitized: string;
}
/** Record of a hash collision */
export interface HashCollision {
sanitized: string;
originals: string[];
}
/**
* Bidirectional mapper for tool name sanitization.
*
@@ -52,6 +58,9 @@ export class ToolNameMapper {
/** List of all changes made during registration */
private changes: SanitizationChange[] = [];
/** List of detected hash collisions */
private collisions: HashCollision[] = [];
/**
* Register tools and sanitize their names.
* Stores mapping for later restoration.
@@ -64,6 +73,23 @@ export class ToolNameMapper {
const result: SanitizeResult = sanitizeToolName(tool.name);
if (result.changed) {
// Check for collision: sanitized name already maps to different original
const existingOriginal = this.mapping.get(result.sanitized);
if (existingOriginal && existingOriginal !== tool.name) {
// Record collision
const existing = this.collisions.find((c) => c.sanitized === result.sanitized);
if (existing) {
if (!existing.originals.includes(tool.name)) {
existing.originals.push(tool.name);
}
} else {
this.collisions.push({
sanitized: result.sanitized,
originals: [existingOriginal, tool.name],
});
}
}
// Store mapping: sanitized → original
this.mapping.set(result.sanitized, tool.name);
this.changes.push({
@@ -143,11 +169,27 @@ export class ToolNameMapper {
return this.changes.length;
}
/**
* Check if any hash collisions were detected.
* Collisions occur when multiple original names map to the same sanitized name.
*/
hasCollisions(): boolean {
return this.collisions.length > 0;
}
/**
* Get all detected hash collisions for logging/warning.
*/
getCollisions(): HashCollision[] {
return [...this.collisions];
}
/**
* Clear all mappings. Call between requests.
*/
clear(): void {
this.mapping.clear();
this.changes = [];
this.collisions = [];
}
}
+18 -2
View File
@@ -13,6 +13,7 @@ import * as http from 'http';
import * as https from 'https';
import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
import { URL } from 'url';
import { ToolNameMapper, type Tool, type ContentBlock } from './tool-name-mapper';
import { getCcsDir } from '../utils/config-manager';
@@ -64,9 +65,13 @@ export class ToolSanitizationProxy {
if (!fs.existsSync(logsDir)) {
fs.mkdirSync(logsDir, { recursive: true });
}
} catch {
} catch (err) {
// Fallback to temp directory if logs dir creation fails
const os = require('os');
if (this.debugMode) {
console.error(
`[tool-sanitization-proxy] Failed to create logs dir: ${(err as Error).message}`
);
}
return path.join(os.tmpdir(), 'tool-sanitization-proxy.log');
}
@@ -213,6 +218,17 @@ export class ToolSanitizationProxy {
}
this.log(`Sanitized ${changes.length} tool name(s)`);
}
// Warn about hash collisions (multiple originals → same sanitized)
if (mapper.hasCollisions()) {
const collisions = mapper.getCollisions();
for (const collision of collisions) {
this.writeLog(
'warn',
`[tool-sanitization-proxy] Hash collision detected: ${collision.originals.join(', ')} → "${collision.sanitized}"`
);
}
}
}
// Check if streaming is requested