From 5985e1d8b9e9b386da7aa493cd9f0e5227c89686 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 25 Apr 2026 11:04:00 +0100 Subject: [PATCH] test: speed up import-heavy tests --- scripts/check-architecture-smells.mjs | 147 +++++------- .../check-extension-plugin-sdk-boundary.mjs | 47 +--- .../lib/extension-import-boundary-checker.mjs | 45 ++-- scripts/lib/guard-inventory-utils.mjs | 116 ++++++++++ src/infra/node-pairing.test.ts | 100 ++++----- src/plugin-sdk/persistent-dedupe.test.ts | 69 ++---- src/plugins/bundle-commands.test.ts | 212 +++++++++++------- test/extension-import-boundaries.test.ts | 21 ++ 8 files changed, 415 insertions(+), 342 deletions(-) diff --git a/scripts/check-architecture-smells.mjs b/scripts/check-architecture-smells.mjs index 29c96527faf..31764021369 100644 --- a/scripts/check-architecture-smells.mjs +++ b/scripts/check-architecture-smells.mjs @@ -1,21 +1,19 @@ #!/usr/bin/env node +import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import ts from "typescript"; import { BUNDLED_PLUGIN_PATH_PREFIX } from "./lib/bundled-plugin-paths.mjs"; import { - collectTypeScriptInventory, + collectModuleReferencesFromSource, normalizeRepoPath, resolveRepoSpecifier, - visitModuleSpecifiers, writeLine, } from "./lib/guard-inventory-utils.mjs"; import { collectTypeScriptFilesFromRoots, resolveSourceRoots, runAsScript, - toLine, } from "./lib/ts-guard-utils.mjs"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); @@ -37,7 +35,7 @@ function pushEntry(entries, entry) { entries.push(entry); } -function scanPluginSdkExtensionFacadeSmells(sourceFile, filePath) { +function scanPluginSdkExtensionFacadeSmells(source, filePath) { const relativeFile = normalizeRepoPath(repoRoot, filePath); if (!relativeFile.startsWith("src/plugin-sdk/")) { return []; @@ -45,28 +43,28 @@ function scanPluginSdkExtensionFacadeSmells(sourceFile, filePath) { const entries = []; - visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { + for (const { kind, line, specifier } of collectModuleReferencesFromSource(source)) { if (kind !== "export") { - return; + continue; } const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); if (!resolvedPath?.startsWith(BUNDLED_PLUGIN_PATH_PREFIX)) { - return; + continue; } pushEntry(entries, { category: "plugin-sdk-extension-facade", file: relativeFile, - line: toLine(sourceFile, specifierNode), + line, kind, specifier, resolvedPath, reason: "plugin-sdk public surface re-exports extension-owned implementation", }); - }); + } return entries; } -function scanRuntimeTypeImplementationSmells(sourceFile, filePath) { +function scanRuntimeTypeImplementationSmells(source, filePath) { const relativeFile = normalizeRepoPath(repoRoot, filePath); if (!/^src\/plugins\/runtime\/types(?:-[^/]+)?\.ts$/.test(relativeFile)) { return []; @@ -74,39 +72,32 @@ function scanRuntimeTypeImplementationSmells(sourceFile, filePath) { const entries = []; - function visit(node) { - if ( - ts.isImportTypeNode(node) && - ts.isLiteralTypeNode(node.argument) && - ts.isStringLiteral(node.argument.literal) - ) { - const specifier = node.argument.literal.text; - const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); - if ( - resolvedPath && - (/^src\/plugins\/runtime\/runtime-[^/]+\.ts$/.test(resolvedPath) || - /^extensions\/[^/]+\/runtime-api\.[^/]+$/.test(resolvedPath)) - ) { - pushEntry(entries, { - category: "runtime-type-implementation-edge", - file: relativeFile, - line: toLine(sourceFile, node.argument.literal), - kind: "import-type", - specifier, - resolvedPath, - reason: "runtime type file references implementation shim directly", - }); - } + for (const { kind, line, specifier } of collectModuleReferencesFromSource(source)) { + if (kind !== "dynamic-import") { + continue; + } + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); + if ( + resolvedPath && + (/^src\/plugins\/runtime\/runtime-[^/]+\.ts$/.test(resolvedPath) || + /^extensions\/[^/]+\/runtime-api\.[^/]+$/.test(resolvedPath)) + ) { + pushEntry(entries, { + category: "runtime-type-implementation-edge", + file: relativeFile, + line, + kind: "import-type", + specifier, + resolvedPath, + reason: "runtime type file references implementation shim directly", + }); } - - ts.forEachChild(node, visit); } - visit(sourceFile); return entries; } -function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { +function scanRuntimeServiceLocatorSmells(source, filePath) { const relativeFile = normalizeRepoPath(repoRoot, filePath); if ( !relativeFile.startsWith("src/plugin-sdk/") && @@ -120,47 +111,25 @@ function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { const runtimeStoreCalls = []; const mutableStateNodes = []; - for (const statement of sourceFile.statements) { - if (ts.isFunctionDeclaration(statement) && statement.name) { - const isExported = statement.modifiers?.some( - (modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword, - ); - if (isExported) { - exportedNames.add(statement.name.text); - } - } else if (ts.isVariableStatement(statement)) { - const isExported = statement.modifiers?.some( - (modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword, - ); - for (const declaration of statement.declarationList.declarations) { - if (ts.isIdentifier(declaration.name) && isExported) { - exportedNames.add(declaration.name.text); - } - if ( - !isExported && - (statement.declarationList.flags & ts.NodeFlags.Let) !== 0 && - ts.isIdentifier(declaration.name) - ) { - mutableStateNodes.push(declaration.name); - } - } + const lines = source.split(/\r?\n/); + for (const [index, line] of lines.entries()) { + const lineNumber = index + 1; + const exportedFunction = line.match(/^\s*export\s+function\s+([A-Za-z_$][\w$]*)/); + if (exportedFunction) { + exportedNames.add(exportedFunction[1]); + } + const exportedVariable = line.match(/^\s*export\s+(?:const|let|var)\s+([A-Za-z_$][\w$]*)/); + if (exportedVariable) { + exportedNames.add(exportedVariable[1]); + } + for (const mutableMatch of line.matchAll(/^\s*let\s+([A-Za-z_$][\w$]*)/g)) { + mutableStateNodes.push({ line: lineNumber, text: mutableMatch[1] }); + } + if (line.includes("createPluginRuntimeStore")) { + runtimeStoreCalls.push({ line: lineNumber }); } } - function visit(node) { - if ( - ts.isCallExpression(node) && - ts.isIdentifier(node.expression) && - node.expression.text === "createPluginRuntimeStore" - ) { - runtimeStoreCalls.push(node.expression); - } - - ts.forEachChild(node, visit); - } - - visit(sourceFile); - const getterNames = [...exportedNames].filter((name) => /^get[A-Z]/.test(name)); const setterNames = [...exportedNames].filter((name) => /^set[A-Z]/.test(name)); @@ -169,7 +138,7 @@ function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { pushEntry(entries, { category: "runtime-service-locator", file: relativeFile, - line: toLine(sourceFile, callNode), + line: callNode.line, kind: "runtime-store", specifier: "createPluginRuntimeStore", resolvedPath: relativeFile, @@ -183,7 +152,7 @@ function scanRuntimeServiceLocatorSmells(sourceFile, filePath) { pushEntry(entries, { category: "runtime-service-locator", file: relativeFile, - line: toLine(sourceFile, identifier), + line: identifier.line, kind: "mutable-state", specifier: identifier.text, resolvedPath: relativeFile, @@ -201,18 +170,16 @@ export async function collectArchitectureSmells() { const files = (await collectTypeScriptFilesFromRoots(scanRoots)).toSorted((left, right) => normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), ); - return await collectTypeScriptInventory({ - ts, - files, - compareEntries, - collectEntries(sourceFile, filePath) { - return [ - ...scanPluginSdkExtensionFacadeSmells(sourceFile, filePath), - ...scanRuntimeTypeImplementationSmells(sourceFile, filePath), - ...scanRuntimeServiceLocatorSmells(sourceFile, filePath), - ]; - }, - }); + const entriesByFile = await Promise.all( + files.map(async (filePath) => { + const source = await fs.readFile(filePath, "utf8"); + const entries = scanPluginSdkExtensionFacadeSmells(source, filePath); + entries.push(...scanRuntimeTypeImplementationSmells(source, filePath)); + entries.push(...scanRuntimeServiceLocatorSmells(source, filePath)); + return entries; + }), + ); + return entriesByFile.flat().toSorted(compareEntries); })(); try { return await architectureSmellsPromise; diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index d88709b1bbf..7e552a13454 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -3,13 +3,13 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import ts from "typescript"; import { BUNDLED_PLUGIN_PATH_PREFIX, BUNDLED_PLUGIN_ROOT_DIR, } from "./lib/bundled-plugin-paths.mjs"; import { classifyBundledExtensionSourcePath } from "./lib/extension-source-classifier.mjs"; import { + collectModuleReferencesFromSource, diffInventoryEntries, normalizeRepoPath, resolveRepoSpecifier, @@ -107,7 +107,7 @@ async function collectExtensionModuleReferences() { } return { filePath, - references: collectModuleReferences(source), + references: collectModuleReferencesFromSource(source), }; }), ); @@ -125,49 +125,6 @@ function mayContainModuleSpecifier(source) { ); } -function collectModuleReferences(source) { - const lineStarts = computeLineStarts(source); - return ts.preProcessFile(source, true, true).importedFiles.map((reference) => ({ - kind: inferModuleReferenceKind(source, reference.pos), - line: lineFromPosition(lineStarts, reference.pos), - specifier: reference.fileName, - })); -} - -function computeLineStarts(source) { - const lineStarts = [0]; - for (let index = 0; index < source.length; index += 1) { - if (source.charCodeAt(index) === 10) { - lineStarts.push(index + 1); - } - } - return lineStarts; -} - -function lineFromPosition(lineStarts, position) { - let low = 0; - let high = lineStarts.length - 1; - while (low <= high) { - const middle = Math.floor((low + high) / 2); - if (lineStarts[middle] <= position) { - low = middle + 1; - } else { - high = middle - 1; - } - } - return high + 1; -} - -function inferModuleReferenceKind(source, specifierStart) { - const importIndex = source.lastIndexOf("import", specifierStart); - const exportIndex = source.lastIndexOf("export", specifierStart); - if (exportIndex > importIndex) { - return "export"; - } - const importPrefix = source.slice(importIndex, specifierStart); - return /\bimport\s*\(\s*["']?$/.test(importPrefix) ? "dynamic-import" : "import"; -} - function resolveExtensionRoot(filePath) { const relativePath = normalizeRepoPath(repoRoot, filePath); const segments = relativePath.split("/"); diff --git a/scripts/lib/extension-import-boundary-checker.mjs b/scripts/lib/extension-import-boundary-checker.mjs index 9350062f621..d854ded1094 100644 --- a/scripts/lib/extension-import-boundary-checker.mjs +++ b/scripts/lib/extension-import-boundary-checker.mjs @@ -1,19 +1,17 @@ -import ts from "typescript"; +import { promises as fs } from "node:fs"; import { BUNDLED_PLUGIN_PATH_PREFIX } from "./bundled-plugin-paths.mjs"; import { - collectTypeScriptInventory, + collectModuleReferencesFromSource, createCachedAsync, formatGroupedInventoryHuman, normalizeRepoPath, resolveRepoSpecifier, - visitModuleSpecifiers, writeLine, } from "./guard-inventory-utils.mjs"; import { collectTypeScriptFilesFromRoots, resolveRepoRoot, resolveSourceRoots, - toLine, } from "./ts-guard-utils.mjs"; const repoRoot = resolveRepoRoot(import.meta.url); @@ -38,27 +36,29 @@ function classifyResolvedExtensionReason(kind, boundaryLabel) { return `${verb} bundled plugin file from ${boundaryLabel} boundary`; } -function scanImportBoundaryViolations(sourceFile, filePath, boundaryLabel, allowResolvedPath) { +function scanImportBoundaryViolations(source, filePath, boundaryLabel, allowResolvedPath) { const entries = []; const relativeFile = normalizeRepoPath(repoRoot, filePath); - visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { + for (const reference of collectModuleReferencesFromSource(source)) { + const kind = reference.kind; + const specifier = reference.specifier; const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); if (!resolvedPath?.startsWith(BUNDLED_PLUGIN_PATH_PREFIX)) { - return; + continue; } if (allowResolvedPath?.(resolvedPath, { kind, specifier, file: relativeFile })) { - return; + continue; } entries.push({ file: relativeFile, - line: toLine(sourceFile, specifierNode), + line: reference.line, kind, specifier, resolvedPath, reason: classifyResolvedExtensionReason(kind, boundaryLabel), }); - }); + } return entries; } @@ -72,22 +72,25 @@ export function createExtensionImportBoundaryChecker(params) { .toSorted((left, right) => normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), ); - return await collectTypeScriptInventory({ - ts, - files, - compareEntries, - collectEntries(sourceFile, filePath) { + const entriesByFile = await Promise.all( + files.map(async (filePath) => { + const source = await fs.readFile(filePath, "utf8"); + if ( + params.skipSourcesWithoutBundledPluginPrefix && + !source.includes(BUNDLED_PLUGIN_PATH_PREFIX) + ) { + return []; + } return scanImportBoundaryViolations( - sourceFile, + source, filePath, params.boundaryLabel, params.allowResolvedPath, ); - }, - shouldParseSource: params.skipSourcesWithoutBundledPluginPrefix - ? (source) => source.includes(BUNDLED_PLUGIN_PATH_PREFIX) - : undefined, - }); + }), + ); + const inventory = entriesByFile.flat(); + return inventory.toSorted(compareEntries); }); async function main(argv = process.argv.slice(2), io) { diff --git a/scripts/lib/guard-inventory-utils.mjs b/scripts/lib/guard-inventory-utils.mjs index a6ee41f82f6..2b60a9da4de 100644 --- a/scripts/lib/guard-inventory-utils.mjs +++ b/scripts/lib/guard-inventory-utils.mjs @@ -75,6 +75,122 @@ export function writeLine(stream, text) { stream.write(`${text}\n`); } +export function collectModuleReferencesFromSource(source) { + const lineStarts = computeLineStarts(source); + const isCodePosition = createCodePositionChecker(source); + const references = []; + const push = (kind, specifier, position, syntaxPosition) => { + if (!isCodePosition(syntaxPosition)) { + return; + } + references.push({ + kind, + line: lineFromPosition(lineStarts, position), + specifier, + }); + }; + + for (const match of source.matchAll(/\bimport\s*\(\s*(["'])([^"']+)\1/g)) { + push("dynamic-import", match[2], match.index + match[0].lastIndexOf(match[1]), match.index); + } + for (const match of source.matchAll(/^\s*import\s*(["'])([^"']+)\1/gm)) { + push( + "import", + match[2], + match.index + match[0].lastIndexOf(match[1]), + match.index + match[0].indexOf("import"), + ); + } + for (const match of source.matchAll( + /^\s*(import|export)\s+(?:type\s+)?[^;"']*?\bfrom\s*(["'])([^"']+)\2/gm, + )) { + push( + match[1], + match[3], + match.index + match[0].lastIndexOf(match[2]), + match.index + match[0].indexOf(match[1]), + ); + } + + return references.toSorted( + (left, right) => + left.line - right.line || + left.kind.localeCompare(right.kind) || + left.specifier.localeCompare(right.specifier), + ); +} + +function createCodePositionChecker(source) { + const codePositions = new Uint8Array(source.length); + + for (let index = 0; index < source.length; index += 1) { + const char = source[index]; + const next = source[index + 1]; + + if (char === "/" && next === "/") { + index += 2; + while (index < source.length && source.charCodeAt(index) !== 10) { + index += 1; + } + index -= 1; + continue; + } + + if (char === "/" && next === "*") { + index += 2; + while (index < source.length && !(source[index] === "*" && source[index + 1] === "/")) { + index += 1; + } + index += 1; + continue; + } + + if (char === "'" || char === '"' || char === "`") { + const quote = char; + index += 1; + while (index < source.length) { + if (source[index] === "\\") { + index += 2; + continue; + } + if (source[index] === quote) { + break; + } + index += 1; + } + continue; + } + + codePositions[index] = 1; + } + + return (position) => codePositions[position] === 1; +} + +function computeLineStarts(source) { + const lineStarts = [0]; + for (let index = 0; index < source.length; index += 1) { + if (source.charCodeAt(index) === 10) { + lineStarts.push(index + 1); + } + } + return lineStarts; +} + +function lineFromPosition(lineStarts, position) { + let low = 0; + let high = lineStarts.length - 1; + while (low <= high) { + const middle = Math.floor((low + high) / 2); + if (lineStarts[middle] <= position) { + low = middle + 1; + } else { + high = middle - 1; + } + } + return high + 1; +} + export function createCachedAsync(factory) { let cachedPromise = null; return async function getCachedValue() { diff --git a/src/infra/node-pairing.test.ts b/src/infra/node-pairing.test.ts index ea4f2ae727b..eba5d95294d 100644 --- a/src/infra/node-pairing.test.ts +++ b/src/infra/node-pairing.test.ts @@ -45,7 +45,7 @@ describe("node pairing tokens", () => { await tempDirs.cleanup(); }); - test("reuses existing pending requests for the same node", async () => { + test("reuses and refreshes pending requests", async () => { await withNodePairingDir(async (baseDir) => { const first = await requestNodePairing( { @@ -65,32 +65,28 @@ describe("node pairing tokens", () => { expect(first.created).toBe(true); expect(second.created).toBe(false); expect(second.request.requestId).toBe(first.request.requestId); - }); - }); - test("refreshes pending requests with newer commands", async () => { - await withNodePairingDir(async (baseDir) => { - const first = await requestNodePairing( + const commandFirst = await requestNodePairing( { - nodeId: "node-1", + nodeId: "node-2", platform: "darwin", commands: ["canvas.snapshot"], }, baseDir, ); - const second = await requestNodePairing( + const commandSecond = await requestNodePairing( { - nodeId: "node-1", + nodeId: "node-2", platform: "darwin", displayName: "Updated Node", commands: ["canvas.snapshot", "system.run"], }, baseDir, ); - const third = await requestNodePairing( + const commandThird = await requestNodePairing( { - nodeId: "node-1", + nodeId: "node-2", platform: "darwin", displayName: "Updated Node", commands: ["canvas.snapshot", "system.run", "system.which"], @@ -98,12 +94,36 @@ describe("node pairing tokens", () => { baseDir, ); - expect(second.created).toBe(false); - expect(second.request.requestId).toBe(first.request.requestId); - expect(third.created).toBe(false); - expect(third.request.requestId).toBe(second.request.requestId); - expect(third.request.displayName).toBe("Updated Node"); - expect(third.request.commands).toEqual(["canvas.snapshot", "system.run", "system.which"]); + expect(commandSecond.created).toBe(false); + expect(commandSecond.request.requestId).toBe(commandFirst.request.requestId); + expect(commandThird.created).toBe(false); + expect(commandThird.request.requestId).toBe(commandSecond.request.requestId); + expect(commandThird.request.displayName).toBe("Updated Node"); + expect(commandThird.request.commands).toEqual([ + "canvas.snapshot", + "system.run", + "system.which", + ]); + + await requestNodePairing( + { + nodeId: "node-3", + platform: "darwin", + commands: ["canvas.present"], + }, + baseDir, + ); + + await expect(listNodePairing(baseDir)).resolves.toEqual({ + pending: expect.arrayContaining([ + expect.objectContaining({ + nodeId: "node-3", + commands: ["canvas.present"], + requiredApproveScopes: ["operator.pairing", "operator.write"], + }), + ]), + paired: [], + }); }); }); @@ -130,9 +150,9 @@ describe("node pairing tokens", () => { }); }); - test("requires operator.admin to approve system.run node commands", async () => { + test("requires the right scopes to approve node requests", async () => { await withNodePairingDir(async (baseDir) => { - const request = await requestNodePairing( + const systemRunRequest = await requestNodePairing( { nodeId: "node-1", platform: "darwin", @@ -143,7 +163,7 @@ describe("node pairing tokens", () => { await expect( approveNodePairing( - request.request.requestId, + systemRunRequest.request.requestId, { callerScopes: ["operator.pairing"] }, baseDir, ), @@ -152,62 +172,34 @@ describe("node pairing tokens", () => { missingScope: "operator.admin", }); await expect(getPairedNode("node-1", baseDir)).resolves.toBeNull(); - }); - }); - test("requires operator.pairing to approve commandless node requests", async () => { - await withNodePairingDir(async (baseDir) => { - const request = await requestNodePairing( + const commandlessRequest = await requestNodePairing( { - nodeId: "node-1", + nodeId: "node-2", platform: "darwin", }, baseDir, ); await expect( - approveNodePairing(request.request.requestId, { callerScopes: [] }, baseDir), + approveNodePairing(commandlessRequest.request.requestId, { callerScopes: [] }, baseDir), ).resolves.toEqual({ status: "forbidden", missingScope: "operator.pairing", }); await expect( approveNodePairing( - request.request.requestId, + commandlessRequest.request.requestId, { callerScopes: ["operator.pairing"] }, baseDir, ), ).resolves.toEqual({ - requestId: request.request.requestId, + requestId: commandlessRequest.request.requestId, node: expect.objectContaining({ - nodeId: "node-1", + nodeId: "node-2", commands: undefined, }), }); }); }); - - test("lists pending requests with precomputed approval scopes", async () => { - await withNodePairingDir(async (baseDir) => { - await requestNodePairing( - { - nodeId: "node-1", - platform: "darwin", - commands: ["canvas.present"], - }, - baseDir, - ); - - await expect(listNodePairing(baseDir)).resolves.toEqual({ - pending: [ - expect.objectContaining({ - nodeId: "node-1", - commands: ["canvas.present"], - requiredApproveScopes: ["operator.pairing", "operator.write"], - }), - ], - paired: [], - }); - }); - }); }); diff --git a/src/plugin-sdk/persistent-dedupe.test.ts b/src/plugin-sdk/persistent-dedupe.test.ts index ad75e9ebaab..4a46541b341 100644 --- a/src/plugin-sdk/persistent-dedupe.test.ts +++ b/src/plugin-sdk/persistent-dedupe.test.ts @@ -30,18 +30,14 @@ describe("createPersistentDedupe", () => { expect(await second.checkAndRecord("m2", { namespace: "a" })).toBe(false); expect(await second.checkAndRecord("m3", { namespace: "a" })).toBe(true); expect(await second.checkAndRecord("m1", { namespace: "b" })).toBe(true); - }); - it("guards concurrent calls for the same key", async () => { - const root = await createTempDir("openclaw-dedupe-"); - const dedupe = createDedupe(root, { ttlMs: 10_000 }); - - const [first, second] = await Promise.all([ - dedupe.checkAndRecord("race-key", { namespace: "feishu" }), - dedupe.checkAndRecord("race-key", { namespace: "feishu" }), + const raceDedupe = createDedupe(root, { ttlMs: 10_000 }); + const [raceFirst, raceSecond] = await Promise.all([ + raceDedupe.checkAndRecord("race-key", { namespace: "feishu" }), + raceDedupe.checkAndRecord("race-key", { namespace: "feishu" }), ]); - expect(first).toBe(true); - expect(second).toBe(false); + expect(raceFirst).toBe(true); + expect(raceSecond).toBe(false); }); it("falls back to memory-only behavior on disk errors", async () => { @@ -56,43 +52,25 @@ describe("createPersistentDedupe", () => { expect(await dedupe.checkAndRecord("memory-only", { namespace: "x" })).toBe(false); }); - it.each([ - { - name: "returns 0 when no disk file exists", - setup: async (root: string) => createDedupe(root, { ttlMs: 10_000 }), - namespace: "nonexistent", - expectedLoaded: 0, - verify: async () => undefined, - }, - { - name: "skips expired entries", - setup: async (root: string) => { - const writer = createDedupe(root, { ttlMs: 1000 }); - const oldNow = Date.now() - 2000; - expect(await writer.checkAndRecord("old-msg", { namespace: "acct", now: oldNow })).toBe( - true, - ); - expect(await writer.checkAndRecord("new-msg", { namespace: "acct" })).toBe(true); - return createDedupe(root, { ttlMs: 1000 }); - }, - namespace: "acct", - expectedLoaded: 1, - verify: async (reader: ReturnType) => { - expect(await reader.checkAndRecord("old-msg", { namespace: "acct" })).toBe(true); - expect(await reader.checkAndRecord("new-msg", { namespace: "acct" })).toBe(false); - }, - }, - ])("warmup $name", async ({ setup, namespace, expectedLoaded, verify }) => { + it("warms empty namespaces and skips expired disk entries", async () => { const root = await createTempDir("openclaw-dedupe-"); - const reader = await setup(root); - const loaded = await reader.warmup(namespace); - expect(loaded).toBe(expectedLoaded); - await verify(reader); + const emptyReader = createDedupe(root, { ttlMs: 10_000 }); + expect(await emptyReader.warmup("nonexistent")).toBe(0); + + const writer = createDedupe(root, { ttlMs: 1000 }); + const oldNow = Date.now() - 2000; + expect(await writer.checkAndRecord("old-msg", { namespace: "acct", now: oldNow })).toBe(true); + expect(await writer.checkAndRecord("new-msg", { namespace: "acct" })).toBe(true); + + const reader = createDedupe(root, { ttlMs: 1000 }); + expect(await reader.warmup("acct")).toBe(1); + expect(await reader.checkAndRecord("old-msg", { namespace: "acct" })).toBe(true); + expect(await reader.checkAndRecord("new-msg", { namespace: "acct" })).toBe(false); }); }); describe("createClaimableDedupe", () => { - it("mirrors concurrent in-flight duplicates and records on commit", async () => { + it("mirrors in-flight duplicates, serializes races, and records on commit", async () => { const dedupe = createClaimableDedupe({ ttlMs: 10_000, memoryMaxSize: 100, @@ -108,13 +86,6 @@ describe("createClaimableDedupe", () => { await expect(duplicate.pending).resolves.toBe(true); } await expect(dedupe.claim("line:evt-1")).resolves.toEqual({ kind: "duplicate" }); - }); - - it("serializes concurrent first-claim races onto one in-flight owner", async () => { - const dedupe = createClaimableDedupe({ - ttlMs: 10_000, - memoryMaxSize: 100, - }); const claims = await Promise.all([dedupe.claim("line:race-1"), dedupe.claim("line:race-1")]); expect(claims.filter((claim) => claim.kind === "claimed")).toHaveLength(1); diff --git a/src/plugins/bundle-commands.test.ts b/src/plugins/bundle-commands.test.ts index 09fb9668d70..eb5a106ccfd 100644 --- a/src/plugins/bundle-commands.test.ts +++ b/src/plugins/bundle-commands.test.ts @@ -1,38 +1,78 @@ -import { afterEach, describe, expect, it } from "vitest"; -import { loadEnabledClaudeBundleCommands } from "./bundle-commands.js"; -import { - createEnabledPluginEntries, - createBundleMcpTempHarness, - withBundleHomeEnv, - writeBundleTextFiles, - writeClaudeBundleManifest, -} from "./bundle-mcp.test-support.js"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { captureEnv } from "../test-utils/env.js"; +import type { PluginManifestRecord } from "./manifest-registry.js"; -const tempHarness = createBundleMcpTempHarness(); +const mocks = vi.hoisted(() => ({ + plugins: [] as PluginManifestRecord[], +})); + +vi.mock("./manifest-registry.js", () => ({ + loadPluginManifestRegistry: () => ({ diagnostics: [], plugins: mocks.plugins }), +})); + +vi.mock("./config-state.js", () => ({ + hasExplicitPluginConfig: (plugins?: { entries?: Record }) => + Boolean(plugins?.entries && Object.keys(plugins.entries).length > 0), + normalizePluginsConfig: (plugins?: unknown) => plugins, + resolveEffectivePluginActivationState: (params: { + config?: { entries?: Record }; + id: string; + }) => ({ + activated: params.config?.entries?.[params.id]?.enabled !== false, + }), +})); + +const { loadEnabledClaudeBundleCommands } = await import("./bundle-commands.js"); + +const tempDirs: string[] = []; afterEach(async () => { - await tempHarness.cleanup(); + mocks.plugins = []; + await Promise.all(tempDirs.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }))); }); +async function createTempDir(prefix: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +function resolveBundlePluginRoot(homeDir: string, pluginId: string) { + return path.join(homeDir, ".openclaw", "extensions", pluginId); +} + async function writeClaudeBundleCommandFixture(params: { homeDir: string; pluginId: string; commands: Array<{ relativePath: string; contents: string[] }>; }) { - const pluginRoot = await writeClaudeBundleManifest({ - homeDir: params.homeDir, - pluginId: params.pluginId, - manifest: { name: params.pluginId }, - }); - await writeBundleTextFiles( - pluginRoot, - Object.fromEntries( - params.commands.map((command) => [ - command.relativePath, - [...command.contents, ""].join("\n"), - ]), - ), + const pluginRoot = resolveBundlePluginRoot(params.homeDir, params.pluginId); + await fs.mkdir(path.join(pluginRoot, ".claude-plugin"), { recursive: true }); + await fs.writeFile( + path.join(pluginRoot, ".claude-plugin", "plugin.json"), + `${JSON.stringify({ name: params.pluginId }, null, 2)}\n`, + "utf-8", ); + await Promise.all( + params.commands.map(async (command) => { + const filePath = path.join(pluginRoot, command.relativePath); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, [...command.contents, ""].join("\n"), "utf-8"); + }), + ); + mocks.plugins = [ + { + id: params.pluginId, + format: "bundle", + bundleFormat: "claude", + bundleCapabilities: ["commands"], + origin: "global", + rootDir: pluginRoot, + } as PluginManifestRecord, + ]; } function expectEnabledClaudeBundleCommands( @@ -51,65 +91,71 @@ function expectEnabledClaudeBundleCommands( describe("loadEnabledClaudeBundleCommands", () => { it("loads enabled Claude bundle markdown commands and skips disabled-model-invocation entries", async () => { - await withBundleHomeEnv( - tempHarness, - "openclaw-bundle-commands", - async ({ homeDir, workspaceDir }) => { - await writeClaudeBundleCommandFixture({ - homeDir, + const env = captureEnv(["HOME", "USERPROFILE", "OPENCLAW_HOME", "OPENCLAW_STATE_DIR"]); + try { + const homeDir = await createTempDir("openclaw-bundle-commands-home-"); + const workspaceDir = await createTempDir("openclaw-bundle-commands-workspace-"); + process.env.HOME = homeDir; + process.env.USERPROFILE = homeDir; + delete process.env.OPENCLAW_HOME; + delete process.env.OPENCLAW_STATE_DIR; + + await writeClaudeBundleCommandFixture({ + homeDir, + pluginId: "compound-bundle", + commands: [ + { + relativePath: "commands/office-hours.md", + contents: [ + "---", + "description: Help with scoping and architecture", + "---", + "Give direct engineering advice.", + ], + }, + { + relativePath: "commands/workflows/review.md", + contents: [ + "---", + "name: workflows:review", + "description: Run a structured review", + "---", + "Review the code. $ARGUMENTS", + ], + }, + { + relativePath: "commands/disabled.md", + contents: ["---", "disable-model-invocation: true", "---", "Do not load me."], + }, + ], + }); + + const commands = loadEnabledClaudeBundleCommands({ + workspaceDir, + cfg: { + plugins: { + entries: { "compound-bundle": { enabled: true } }, + }, + }, + }); + + expectEnabledClaudeBundleCommands(commands, [ + { pluginId: "compound-bundle", - commands: [ - { - relativePath: "commands/office-hours.md", - contents: [ - "---", - "description: Help with scoping and architecture", - "---", - "Give direct engineering advice.", - ], - }, - { - relativePath: "commands/workflows/review.md", - contents: [ - "---", - "name: workflows:review", - "description: Run a structured review", - "---", - "Review the code. $ARGUMENTS", - ], - }, - { - relativePath: "commands/disabled.md", - contents: ["---", "disable-model-invocation: true", "---", "Do not load me."], - }, - ], - }); - - const commands = loadEnabledClaudeBundleCommands({ - workspaceDir, - cfg: { - plugins: { - entries: createEnabledPluginEntries(["compound-bundle"]), - }, - }, - }); - - expectEnabledClaudeBundleCommands(commands, [ - { - pluginId: "compound-bundle", - rawName: "office-hours", - description: "Help with scoping and architecture", - promptTemplate: "Give direct engineering advice.", - }, - { - pluginId: "compound-bundle", - rawName: "workflows:review", - description: "Run a structured review", - promptTemplate: "Review the code. $ARGUMENTS", - }, - ]); - expect(commands.some((entry) => entry.rawName === "disabled")).toBe(false); - }, - ); + rawName: "office-hours", + description: "Help with scoping and architecture", + promptTemplate: "Give direct engineering advice.", + }, + { + pluginId: "compound-bundle", + rawName: "workflows:review", + description: "Run a structured review", + promptTemplate: "Review the code. $ARGUMENTS", + }, + ]); + expect(commands.some((entry) => entry.rawName === "disabled")).toBe(false); + } finally { + env.restore(); + } }); }); diff --git a/test/extension-import-boundaries.test.ts b/test/extension-import-boundaries.test.ts index 6761be2dfa7..bd782545f08 100644 --- a/test/extension-import-boundaries.test.ts +++ b/test/extension-import-boundaries.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { main as extensionPluginSdkMain } from "../scripts/check-extension-plugin-sdk-boundary.mjs"; import { main as sdkPackageMain } from "../scripts/check-sdk-package-extension-import-boundary.mjs"; import { main as srcExtensionMain } from "../scripts/check-src-extension-import-boundary.mjs"; +import { collectModuleReferencesFromSource } from "../scripts/lib/guard-inventory-utils.mjs"; import { createCapturedIo } from "./helpers/captured-io.js"; const srcJsonOutputPromise = getJsonOutput(srcExtensionMain, ["--json"]); @@ -21,6 +22,26 @@ const relativeOutsidePackageJsonOutputPromise = getJsonOutput(extensionPluginSdk type CapturedIo = ReturnType["io"]; +describe("fast module reference scanner", () => { + it("collects code references without matching comments or strings", () => { + expect( + collectModuleReferencesFromSource(` +// import "./commented"; +const text = 'import("./string")'; +import "./side-effect"; +import type { Example } from "./types"; +export { Example } from "./public"; +await import("./runtime"); +`), + ).toEqual([ + { kind: "import", line: 4, specifier: "./side-effect" }, + { kind: "import", line: 5, specifier: "./types" }, + { kind: "export", line: 6, specifier: "./public" }, + { kind: "dynamic-import", line: 7, specifier: "./runtime" }, + ]); + }); +}); + async function getJsonOutput( main: (argv: string[], io: CapturedIo) => Promise, argv: string[],