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:
Tam Nhu Tran
2026-04-29 17:57:11 -04:00
parent 31dc18657c
commit 218d0cf6c6
4 changed files with 159 additions and 26 deletions
@@ -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);
});
});
});
+39 -21
View File
@@ -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 {