mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 10:16:49 +00:00
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:
@@ -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;
|
||||
|
||||
|
||||
@@ -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 = [];
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user