mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(agents): guard duplicate tool display metadata (#87025)
This commit is contained in:
Josh Avant
@@ -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<string>();
|
||||
for (const sourcePath of toolSources) {
|
||||
@@ -92,3 +108,131 @@ function collectToolNamesFromFile(sourcePath: string, names: Set<string>) {
|
||||
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<string, number[]>();
|
||||
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();
|
||||
}
|
||||
|
||||
72
test/scripts/tool-display.test.ts
Normal file
72
test/scripts/tool-display.test.ts
Normal file
@@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user