diff --git a/scripts/tool-display.ts b/scripts/tool-display.ts index bb3f24db702e..a92bc95dc9cb 100644 --- a/scripts/tool-display.ts +++ b/scripts/tool-display.ts @@ -1,10 +1,12 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import ts from "typescript"; import { TOOL_DISPLAY_CONFIG, type ToolDisplayConfig } from "../src/agents/tool-display-config.js"; const scriptDir = path.dirname(fileURLToPath(import.meta.url)); const repoRoot = path.resolve(scriptDir, ".."); +const configPath = path.join(repoRoot, "src/agents/tool-display-config.ts"); const outputPath = path.join( repoRoot, "apps/shared/OpenClawKit/Sources/OpenClawKit/Resources/tool-display.json", @@ -16,49 +18,63 @@ const toolSources = [ path.join(repoRoot, "src/auto-reply/reply/acp-projector.ts"), ]; -const args = new Set(process.argv.slice(2)); -const shouldCheck = args.has("--check"); -const shouldWrite = args.has("--write"); +type DuplicateToolKey = { + name: string; + lines: number[]; +}; -if (!shouldCheck && !shouldWrite) { - console.error("Usage: node --import tsx scripts/tool-display.ts --check|--write"); - process.exit(1); +export function main(argv = process.argv.slice(2)): number { + const args = new Set(argv); + const shouldCheck = args.has("--check"); + const shouldWrite = args.has("--write"); + + if (!shouldCheck && !shouldWrite) { + console.error("Usage: node --import tsx scripts/tool-display.ts --check|--write"); + return 1; + } + + const duplicateErrors = collectToolDisplayDuplicateErrors({ includeSnapshot: shouldCheck }); + if (duplicateErrors.length > 0) { + console.error(duplicateErrors.join("\n")); + return 1; + } + + const expected = serializeToolDisplayConfig(); + ensureCoreToolCoverage(); + + if (shouldWrite) { + fs.mkdirSync(path.dirname(outputPath), { recursive: true }); + fs.writeFileSync(outputPath, expected); + process.stdout.write(`wrote ${path.relative(repoRoot, outputPath)}\n`); + return 0; + } + + if (!fs.existsSync(path.dirname(outputPath))) { + process.stdout.write( + `skip tool-display snapshot check; missing ${path.relative(repoRoot, path.dirname(outputPath))}\n`, + ); + return 0; + } + + if (!fs.existsSync(outputPath)) { + console.error( + `missing generated snapshot: ${path.relative(repoRoot, outputPath)}\nrun: pnpm tool-display:write`, + ); + return 1; + } + + const actual = fs.readFileSync(outputPath, "utf8"); + if (actual !== expected) { + console.error( + `tool-display snapshot is stale: ${path.relative(repoRoot, outputPath)}\nrun: pnpm tool-display:write`, + ); + return 1; + } + + process.stdout.write("tool-display snapshot is up to date\n"); + return 0; } -const expected = serializeToolDisplayConfig(); -ensureCoreToolCoverage(); - -if (shouldWrite) { - fs.mkdirSync(path.dirname(outputPath), { recursive: true }); - fs.writeFileSync(outputPath, expected); - process.stdout.write(`wrote ${path.relative(repoRoot, outputPath)}\n`); - process.exit(0); -} - -if (!fs.existsSync(path.dirname(outputPath))) { - process.stdout.write( - `skip tool-display snapshot check; missing ${path.relative(repoRoot, path.dirname(outputPath))}\n`, - ); - process.exit(0); -} - -if (!fs.existsSync(outputPath)) { - console.error( - `missing generated snapshot: ${path.relative(repoRoot, outputPath)}\nrun: pnpm tool-display:write`, - ); - process.exit(1); -} - -const actual = fs.readFileSync(outputPath, "utf8"); -if (actual !== expected) { - console.error( - `tool-display snapshot is stale: ${path.relative(repoRoot, outputPath)}\nrun: pnpm tool-display:write`, - ); - process.exit(1); -} - -process.stdout.write("tool-display snapshot is up to date\n"); - function ensureCoreToolCoverage() { const toolNames = new Set(); for (const sourcePath of toolSources) { @@ -92,3 +108,131 @@ function collectToolNamesFromFile(sourcePath: string, names: Set) { function serializeToolDisplayConfig(config: ToolDisplayConfig = TOOL_DISPLAY_CONFIG): string { return `${JSON.stringify(config, null, 2)}\n`; } + +function collectToolDisplayDuplicateErrors(options: { includeSnapshot: boolean }): string[] { + const duplicateErrors: string[] = []; + const configSource = fs.readFileSync(configPath, "utf8"); + const configDuplicates = collectToolDisplayConfigDuplicateKeys(configSource, configPath); + if (configDuplicates.length > 0) { + duplicateErrors.push( + formatDuplicateToolKeyError(path.relative(repoRoot, configPath), configDuplicates), + ); + } + + if (options.includeSnapshot && fs.existsSync(outputPath)) { + const snapshotSource = fs.readFileSync(outputPath, "utf8"); + const snapshotDuplicates = collectToolDisplaySnapshotDuplicateKeys(snapshotSource, outputPath); + if (snapshotDuplicates.length > 0) { + duplicateErrors.push( + formatDuplicateToolKeyError(path.relative(repoRoot, outputPath), snapshotDuplicates), + ); + } + } + return duplicateErrors; +} + +export function collectToolDisplayConfigDuplicateKeys( + source: string, + sourcePath = "src/agents/tool-display-config.ts", +): DuplicateToolKey[] { + const sourceFile = ts.createSourceFile(sourcePath, source, ts.ScriptTarget.Latest, true); + let toolsObject: ts.ObjectLiteralExpression | undefined; + visitToolDisplayConfig(sourceFile, (configObject) => { + toolsObject = findObjectProperty(configObject, "tools"); + }); + return toolsObject ? collectDuplicatePropertyKeys(toolsObject, sourceFile) : []; +} + +export function collectToolDisplaySnapshotDuplicateKeys( + source: string, + sourcePath = "tool-display.json", +): DuplicateToolKey[] { + const sourceFile = ts.parseJsonText(sourcePath, source); + const statement = sourceFile.statements[0]; + if (!statement || !ts.isExpressionStatement(statement)) { + return []; + } + const root = statement.expression; + if (!ts.isObjectLiteralExpression(root)) { + return []; + } + const toolsObject = findObjectProperty(root, "tools"); + return toolsObject ? collectDuplicatePropertyKeys(toolsObject, sourceFile) : []; +} + +export function formatDuplicateToolKeyError( + relativePath: string, + duplicates: DuplicateToolKey[], +): string { + const formatted = duplicates + .map((duplicate) => `${duplicate.name} at lines ${duplicate.lines.join(", ")}`) + .join("; "); + return `tool-display metadata has duplicate tool ids in ${relativePath}: ${formatted}`; +} + +function visitToolDisplayConfig( + node: ts.Node, + onConfig: (configObject: ts.ObjectLiteralExpression) => void, +) { + if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.name.text === "TOOL_DISPLAY_CONFIG" && + node.initializer && + ts.isObjectLiteralExpression(node.initializer) + ) { + onConfig(node.initializer); + return; + } + ts.forEachChild(node, (child) => visitToolDisplayConfig(child, onConfig)); +} + +function findObjectProperty( + object: ts.ObjectLiteralExpression, + propertyName: string, +): ts.ObjectLiteralExpression | undefined { + for (const property of object.properties) { + if ( + ts.isPropertyAssignment(property) && + getPropertyNameText(property.name) === propertyName && + ts.isObjectLiteralExpression(property.initializer) + ) { + return property.initializer; + } + } + return undefined; +} + +function collectDuplicatePropertyKeys( + object: ts.ObjectLiteralExpression, + sourceFile: ts.SourceFile, +): DuplicateToolKey[] { + const keyLines = new Map(); + for (const property of object.properties) { + if (!ts.isPropertyAssignment(property)) { + continue; + } + const name = getPropertyNameText(property.name); + if (!name) { + continue; + } + const line = + sourceFile.getLineAndCharacterOfPosition(property.name.getStart(sourceFile)).line + 1; + keyLines.set(name, [...(keyLines.get(name) ?? []), line]); + } + return [...keyLines.entries()] + .filter(([, lines]) => lines.length > 1) + .map(([name, lines]) => ({ name, lines })) + .toSorted((left, right) => left.name.localeCompare(right.name)); +} + +function getPropertyNameText(name: ts.PropertyName): string | undefined { + if (ts.isIdentifier(name) || ts.isStringLiteral(name) || ts.isNumericLiteral(name)) { + return name.text; + } + return undefined; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = main(); +} diff --git a/test/scripts/tool-display.test.ts b/test/scripts/tool-display.test.ts new file mode 100644 index 000000000000..aaa4d396fdbe --- /dev/null +++ b/test/scripts/tool-display.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { + collectToolDisplayConfigDuplicateKeys, + collectToolDisplaySnapshotDuplicateKeys, + formatDuplicateToolKeyError, +} from "../../scripts/tool-display.ts"; + +describe("tool-display duplicate metadata guard", () => { + it("reports duplicate top-level tool ids in the source config", () => { + const duplicates = collectToolDisplayConfigDuplicateKeys(` + export const TOOL_DISPLAY_CONFIG = { + version: 1, + tools: { + transcripts: {}, + web_search: { + actions: { + status: {}, + }, + }, + transcripts: {}, + }, + }; + `); + + expect(duplicates).toEqual([{ lines: [5, 11], name: "transcripts" }]); + }); + + it("ignores duplicate action ids under separate tool specs", () => { + const duplicates = collectToolDisplayConfigDuplicateKeys(` + export const TOOL_DISPLAY_CONFIG = { + version: 1, + tools: { + browser: { + actions: { + status: {}, + }, + }, + transcripts: { + actions: { + status: {}, + }, + }, + }, + }; + `); + + expect(duplicates).toEqual([]); + }); + + it("reports duplicate top-level tool ids in the generated snapshot", () => { + const duplicates = collectToolDisplaySnapshotDuplicateKeys(`{ + "version": 1, + "tools": { + "transcripts": {}, + "web_search": {}, + "transcripts": {} + } + }`); + + expect(duplicates).toEqual([{ lines: [4, 6], name: "transcripts" }]); + }); + + it("formats duplicate metadata errors with line numbers", () => { + expect( + formatDuplicateToolKeyError("src/agents/tool-display-config.ts", [ + { lines: [10, 20], name: "transcripts" }, + ]), + ).toBe( + "tool-display metadata has duplicate tool ids in src/agents/tool-display-config.ts: transcripts at lines 10, 20", + ); + }); +});