mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-16 04:18:05 +00:00
fix(cliproxy): sanitize invalid tool name chars and disambiguate collisions
sanitizeToolName() fallback now forces valid characters (replaces unsupported chars with underscores, ensures leading letter/underscore) instead of only truncating. registerTools() disambiguates colliding sanitized names with numeric suffixes so restoreToolUse() always maps back to the correct original.
This commit is contained in:
@@ -4,7 +4,7 @@
|
||||
|
||||
const assert = require('assert');
|
||||
|
||||
const { ToolNameMapper } = require('../../../../dist/cliproxy/tool-name-mapper');
|
||||
const { ToolNameMapper } = require('../../../../dist/cliproxy/ai-providers/tool-name-mapper');
|
||||
|
||||
describe('Tool Name Mapper', () => {
|
||||
describe('registerTools', () => {
|
||||
@@ -243,4 +243,62 @@ describe('Tool Name Mapper', () => {
|
||||
assert.strictEqual(mapper.restoreName('foo__bar'), 'foo__bar');
|
||||
});
|
||||
});
|
||||
|
||||
describe('collision handling', () => {
|
||||
it('disambiguates when sanitized name collides with unchanged name', () => {
|
||||
const mapper = new ToolNameMapper();
|
||||
// 'abc__def__def' sanitizes to 'abc__def' (changed)
|
||||
// 'abc__def' is unchanged but collides with the sanitized name
|
||||
const tools = [{ name: 'abc__def__def' }, { name: 'abc__def' }];
|
||||
|
||||
const result = mapper.registerTools(tools);
|
||||
|
||||
assert.strictEqual(result[0].name, 'abc__def');
|
||||
assert.strictEqual(result[1].name, 'abc__def_2');
|
||||
assert.strictEqual(mapper.hasCollisions(), true);
|
||||
});
|
||||
|
||||
it('disambiguates different originals that sanitize to the same name', () => {
|
||||
const mapper = new ToolNameMapper();
|
||||
const tools = [{ name: 'abc__def__def' }, { name: 'abc__def' }];
|
||||
|
||||
const result = mapper.registerTools(tools);
|
||||
|
||||
// 'abc__def__def' → 'abc__def' (deduped)
|
||||
// 'abc__def' → 'abc__def_2' (disambiguated, unchanged but collides)
|
||||
assert.strictEqual(result[0].name, 'abc__def');
|
||||
assert.strictEqual(result[1].name, 'abc__def_2');
|
||||
});
|
||||
|
||||
it('restores both colliding tools to correct originals', () => {
|
||||
const mapper = new ToolNameMapper();
|
||||
const tools = [{ name: 'a__b__b' }, { name: 'a__b' }];
|
||||
|
||||
mapper.registerTools(tools);
|
||||
|
||||
const content = [
|
||||
{ type: 'tool_use', id: '1', name: 'a__b', input: {} },
|
||||
{ type: 'tool_use', id: '2', name: 'a__b_2', input: {} },
|
||||
];
|
||||
|
||||
const restored = mapper.restoreToolUse(content);
|
||||
|
||||
assert.strictEqual(restored[0].name, 'a__b__b');
|
||||
assert.strictEqual(restored[1].name, 'a__b');
|
||||
});
|
||||
|
||||
it('records collision metadata for logging', () => {
|
||||
const mapper = new ToolNameMapper();
|
||||
const tools = [{ name: 'x__y__y' }, { name: 'x__y' }];
|
||||
|
||||
mapper.registerTools(tools);
|
||||
|
||||
assert.strictEqual(mapper.hasCollisions(), true);
|
||||
const collisions = mapper.getCollisions();
|
||||
assert.strictEqual(collisions.length, 1);
|
||||
assert.strictEqual(collisions[0].base, 'x__y');
|
||||
assert.ok(collisions[0].originals.includes('x__y__y'));
|
||||
assert.ok(collisions[0].originals.includes('x__y'));
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -10,7 +10,7 @@ const {
|
||||
smartTruncate,
|
||||
sanitizeToolName,
|
||||
GEMINI_MAX_TOOL_NAME_LENGTH,
|
||||
} = require('../../../../dist/cliproxy/tool-name-sanitizer');
|
||||
} = require('../../../../dist/cliproxy/ai-providers/tool-name-sanitizer');
|
||||
|
||||
describe('Tool Name Sanitizer', () => {
|
||||
describe('GEMINI_MAX_TOOL_NAME_LENGTH', () => {
|
||||
@@ -170,5 +170,38 @@ describe('Tool Name Sanitizer', () => {
|
||||
assert.strictEqual(result.changed, true);
|
||||
assert.strictEqual(result.sanitized, 'gitmcp__plus-pro-components');
|
||||
});
|
||||
|
||||
it('forces valid characters when name contains spaces', () => {
|
||||
const result = sanitizeToolName('has space');
|
||||
|
||||
assert.strictEqual(result.sanitized, 'has_space');
|
||||
assert.strictEqual(result.changed, true);
|
||||
assert.ok(isValidToolName(result.sanitized));
|
||||
});
|
||||
|
||||
it('forces valid characters when name starts with digit', () => {
|
||||
const result = sanitizeToolName('123start');
|
||||
|
||||
assert.ok(result.sanitized.startsWith('_'));
|
||||
assert.strictEqual(result.changed, true);
|
||||
assert.ok(isValidToolName(result.sanitized));
|
||||
});
|
||||
|
||||
it('forces valid characters when name contains symbols', () => {
|
||||
const result = sanitizeToolName('tool@name#here');
|
||||
|
||||
assert.ok(!result.sanitized.includes('@'));
|
||||
assert.ok(!result.sanitized.includes('#'));
|
||||
assert.strictEqual(result.changed, true);
|
||||
assert.ok(isValidToolName(result.sanitized));
|
||||
});
|
||||
|
||||
it('produces valid name for digit-starting long name', () => {
|
||||
const name = '1' + 'a'.repeat(100);
|
||||
const result = sanitizeToolName(name);
|
||||
|
||||
assert.ok(isValidToolName(result.sanitized), `Expected valid name, got: ${result.sanitized}`);
|
||||
assert.ok(result.sanitized.length <= 64);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -41,8 +41,12 @@ export interface SanitizationChange {
|
||||
|
||||
/** Record of a hash collision */
|
||||
export interface HashCollision {
|
||||
sanitized: string;
|
||||
/** The base sanitized name before disambiguation */
|
||||
base: string;
|
||||
/** All original names that collided */
|
||||
originals: string[];
|
||||
/** The sanitized name used (after disambiguation) */
|
||||
sanitized: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -63,7 +67,8 @@ export class ToolNameMapper {
|
||||
|
||||
/**
|
||||
* Register tools and sanitize their names.
|
||||
* Stores mapping for later restoration.
|
||||
* Stores mapping for later restoration. On collision (two originals
|
||||
* sanitizing to the same name), disambiguates with a numeric suffix.
|
||||
*
|
||||
* @param tools Array of tool definitions
|
||||
* @returns Array of tools with sanitized names
|
||||
@@ -71,36 +76,49 @@ export class ToolNameMapper {
|
||||
registerTools(tools: Tool[]): Tool[] {
|
||||
return tools.map((tool) => {
|
||||
const result: SanitizeResult = sanitizeToolName(tool.name);
|
||||
let finalName = result.sanitized;
|
||||
|
||||
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],
|
||||
});
|
||||
// Check for collision regardless of whether the name changed.
|
||||
// An unchanged name can still collide with a previously sanitized name.
|
||||
const existingOriginal = this.mapping.get(finalName);
|
||||
if (existingOriginal && existingOriginal !== tool.name) {
|
||||
// Record collision for logging
|
||||
const existing = this.collisions.find((c) => c.base === finalName);
|
||||
if (existing) {
|
||||
if (!existing.originals.includes(tool.name)) {
|
||||
existing.originals.push(tool.name);
|
||||
}
|
||||
} else {
|
||||
this.collisions.push({
|
||||
base: finalName,
|
||||
originals: [existingOriginal, tool.name],
|
||||
sanitized: finalName,
|
||||
});
|
||||
}
|
||||
|
||||
// Store mapping: sanitized → original
|
||||
this.mapping.set(result.sanitized, tool.name);
|
||||
// Disambiguate: append _N suffix to make unique
|
||||
let suffix = 2;
|
||||
let disambiguated: string;
|
||||
do {
|
||||
disambiguated = `${finalName}_${suffix}`;
|
||||
suffix++;
|
||||
} while (this.mapping.has(disambiguated) && this.mapping.get(disambiguated) !== tool.name);
|
||||
|
||||
finalName = disambiguated;
|
||||
}
|
||||
|
||||
// Store mapping if name changed or was disambiguated
|
||||
if (finalName !== tool.name) {
|
||||
this.mapping.set(finalName, tool.name);
|
||||
this.changes.push({
|
||||
original: tool.name,
|
||||
sanitized: result.sanitized,
|
||||
sanitized: finalName,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
...tool,
|
||||
name: result.sanitized,
|
||||
name: finalName,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
@@ -100,13 +100,34 @@ export function smartTruncate(name: string, maxLen: number = GEMINI_MAX_TOOL_NAM
|
||||
return `${prefix}_${hash}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Force a string to comply with Gemini tool name character rules.
|
||||
* Replaces invalid characters with underscores and ensures the name
|
||||
* starts with a letter or underscore.
|
||||
*/
|
||||
function forceValidChars(name: string): string {
|
||||
// Replace any character not in [a-zA-Z0-9_.:/-] with underscore
|
||||
let fixed = name.replace(/[^a-zA-Z0-9_.:/-]/g, '_');
|
||||
|
||||
// Collapse multiple consecutive underscores from replacement
|
||||
fixed = fixed.replace(/_+/g, '_');
|
||||
|
||||
// Ensure starts with letter or underscore
|
||||
if (fixed.length > 0 && /^[0-9]/.test(fixed)) {
|
||||
fixed = '_' + fixed;
|
||||
}
|
||||
|
||||
return fixed || '_';
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanitize a tool name to comply with Gemini API constraints.
|
||||
*
|
||||
* Process:
|
||||
* 1. Remove duplicate segments (always, as duplicates are likely unintentional)
|
||||
* 2. Truncate with hash if >64 chars
|
||||
* 3. Return result with changed flag
|
||||
* 3. Force valid characters if still invalid
|
||||
* 4. Truncate again if forced-valid name exceeds limit
|
||||
*
|
||||
* @param name The original tool name
|
||||
* @returns Sanitization result with sanitized name and changed flag
|
||||
@@ -121,9 +142,12 @@ export function sanitizeToolName(name: string): SanitizeResult {
|
||||
sanitized = smartTruncate(sanitized);
|
||||
}
|
||||
|
||||
// Step 3: If still invalid after sanitization, apply truncation to original
|
||||
// Step 3: If still invalid, fix characters and truncate if needed
|
||||
if (!isValidToolName(sanitized)) {
|
||||
sanitized = smartTruncate(name);
|
||||
sanitized = forceValidChars(sanitized);
|
||||
if (sanitized.length > GEMINI_MAX_TOOL_NAME_LENGTH) {
|
||||
sanitized = smartTruncate(sanitized);
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user