diff --git a/package.json b/package.json index 10450d65ce3b..50e55576e6fd 100644 --- a/package.json +++ b/package.json @@ -1526,7 +1526,7 @@ "channels:catalog:gen": "node scripts/write-official-channel-catalog.mjs --write", "changed:lanes": "node scripts/changed-lanes.mjs", "check": "node --import tsx scripts/check.mts", - "check:architecture": "pnpm check:import-cycles && pnpm check:madge-import-cycles && pnpm check:deprecated-api-usage && pnpm check:deprecated-jsdoc && pnpm db:kysely:check && pnpm lint:kysely && pnpm check:database-first-legacy-stores", + "check:architecture": "pnpm check:import-cycles && pnpm check:madge-import-cycles && pnpm check:deprecated-api-usage && pnpm check:wrapper-shadowing && pnpm check:deprecated-jsdoc && pnpm db:kysely:check && pnpm lint:kysely && pnpm check:database-first-legacy-stores", "check:base-config-schema": "node --import tsx scripts/generate-base-config-schema.ts --check", "check:bundled-channel-config-metadata": "node --import tsx scripts/generate-bundled-channel-config-metadata.ts --check", "check:changed": "node scripts/check-changed.mjs", @@ -1549,6 +1549,8 @@ "check:runtime-sidecar-loaders": "node --import tsx scripts/check-runtime-sidecar-loaders.mts", "check:static-import-sccs": "pnpm check:madge-import-cycles", "check:temp-path-guardrails": "node --import tsx scripts/check-temp-path-guardrails.ts", + "check:wrapper-shadowing": "node --import tsx scripts/check-wrapper-shadowing.mts", + "check:wrapper-shadowing:gen": "node --import tsx scripts/check-wrapper-shadowing.mts --update-debt-baseline", "check:test-types": "pnpm tsgo:test", "check:timed": "node --import tsx scripts/check-timed.mts", "check:timed:all-types": "node --import tsx scripts/check-timed.mts --include-test-types", diff --git a/scripts/check-changed.mts b/scripts/check-changed.mts index 16870d260908..cad34cf86844 100644 --- a/scripts/check-changed.mts +++ b/scripts/check-changed.mts @@ -101,6 +101,8 @@ const PLUGIN_SDK_SURFACE_PATH_RE = /^(?:package\.json$|src\/plugin-sdk\/|packages\/plugin-sdk\/|scripts\/(?:plugin-sdk-surface-report\.mts|sync-plugin-sdk-exports\.mts|lib\/plugin-sdk-(?:declaration-budget\.mts|deprecated-barrel-subpaths\.json|deprecated-public-subpaths\.json|entries\.mts|entrypoints\.json|private-local-only-subpaths\.json)))/u; const DEPRECATION_HYGIENE_PATH_RE = /^(?:package\.json$|src\/|extensions\/|packages\/|scripts\/(?:check-deprecated-api-usage\.mts$|plugin-boundary-report\.ts$|lib\/plugin-sdk))/u; +const WRAPPER_SHADOWING_PATH_RE = + /^(?:package\.json$|src\/|scripts\/(?:check-(?:export-name-collisions|wrapper-shadowing)\.mts$|lib\/(?:export-name-collision-baseline\.json$|ts-guard-utils\.mts$|wrapper-shadowing-baseline\.json$)))/u; const CANVAS_A2UI_NATIVE_RESOURCE_PATH_RE = /^(?:pnpm-lock\.yaml$|apps\/(?:android\/app\/build\.gradle\.kts$|ios\/project\.yml$|linux\/src-tauri\/(?:build\.rs$|src\/canvas\.rs$)|shared\/OpenClawKit\/Sources\/OpenClawKit\/Resources\/CanvasA2UI\/)|extensions\/canvas\/(?:package\.json$|scripts\/bundle-a2ui\.mjs$|src\/host\/a2ui(?:\/(?:index\.html|a2ui\.bundle\.js|\.bundle\.hash)$|-app\/))|scripts\/(?:bundle-a2ui|sync-native-a2ui)\.mts$)/u; const CONTROL_UI_I18N_VERIFY_PATH_RE = @@ -382,6 +384,13 @@ export function shouldRunDeprecationHygieneChecks(paths: string[]) { ); } +/** Returns whether changed files can alter wrapper-shadowing results. */ +export function shouldRunWrapperShadowingCheck(paths: string[]) { + return paths.some((changedPath) => + WRAPPER_SHADOWING_PATH_RE.test(normalizeChangedPath(changedPath)), + ); +} + export function shouldRunCanvasA2uiNativeResourceCheck(paths: string[]) { return paths.some((changedPath) => CANVAS_A2UI_NATIVE_RESOURCE_PATH_RE.test(normalizeChangedPath(changedPath)), @@ -677,6 +686,9 @@ export function createChangedCheckPlan( // until their scheduled deletion PRs land. add("plugin boundaries", ["plugins:boundary-report:ci"]); } + if (result.lanes.all || shouldRunWrapperShadowingCheck(result.paths)) { + add("wrapper shadowing", ["check:wrapper-shadowing"]); + } if (shouldRunCanvasA2uiNativeResourceCheck(result.paths)) { addCommand( "Canvas A2UI native resource generation", diff --git a/scripts/check-export-name-collisions.mts b/scripts/check-export-name-collisions.mts index 50d1b474af02..3a77daf54528 100644 --- a/scripts/check-export-name-collisions.mts +++ b/scripts/check-export-name-collisions.mts @@ -19,16 +19,35 @@ export type ExportNameCollision = { sdk?: true; }; -type SourceModule = { +export type SourceModule = { content: string; includeDefinitions?: boolean; path: string; }; -type ModuleExports = { +export type ImportedSymbolReference = { + importedName: string; + localName: string; + moduleSpecifier: string; +}; + +export type NamedReExport = { + exportedName: string; + importedName: string; + moduleSpecifier: string; +}; + +export type ExportedValueDefinition = { + importedReferences: ImportedSymbolReference[]; + name: string; +}; + +export type ModuleExports = { definitions: Set; exportedNames: Set; + namedReExports: NamedReExport[]; starExportSpecifiers: string[]; + valueDefinitions: Map; }; const exportNameCollisionSchema = z @@ -75,6 +94,72 @@ function collectBindingNames(name: ts.BindingName, names: Set) { } } +function resolveImportedReference( + expression: ts.Expression, + importedSymbolsByLocalName: ReadonlyMap, + namespaceImportsByLocalName: ReadonlyMap, +) { + const target = unwrapExpression(expression); + if (ts.isIdentifier(target)) { + return importedSymbolsByLocalName.get(target.text); + } + if (!ts.isPropertyAccessExpression(target) && !ts.isElementAccessExpression(target)) { + return undefined; + } + const namespaceName = ts.isPropertyAccessExpression(target) + ? target.name.text + : ts.isElementAccessExpression(target) && + target.argumentExpression && + ts.isStringLiteral(target.argumentExpression) + ? target.argumentExpression.text + : null; + if (!namespaceName || !ts.isIdentifier(target.expression)) { + return undefined; + } + const moduleSpecifier = namespaceImportsByLocalName.get(target.expression.text); + return moduleSpecifier + ? { + importedName: namespaceName, + localName: `${target.expression.text}.${namespaceName}`, + moduleSpecifier, + } + : undefined; +} + +function collectImportedReferences( + node: ts.Node, + importedSymbolsByLocalName: ReadonlyMap, + namespaceImportsByLocalName: ReadonlyMap, +) { + const references = new Map(); + const addReference = (reference: ImportedSymbolReference | undefined) => { + if (reference) { + references.set( + `${reference.moduleSpecifier}\0${reference.importedName}\0${reference.localName}`, + reference, + ); + } + }; + const visit = (current: ts.Node): void => { + if (ts.isCallExpression(current)) { + addReference( + resolveImportedReference( + current.expression, + importedSymbolsByLocalName, + namespaceImportsByLocalName, + ), + ); + } + ts.forEachChild(current, visit); + }; + visit(node); + return [...references.values()].toSorted((left, right) => + `${left.moduleSpecifier}\0${left.importedName}\0${left.localName}`.localeCompare( + `${right.moduleSpecifier}\0${right.importedName}\0${right.localName}`, + ), + ); +} + function parametersAreForwarded( parameters: ts.NodeArray, args: ts.NodeArray, @@ -238,11 +323,15 @@ function isForwardingOnlyConst( export function collectModuleExportNames(content: string, fileName = "source.ts"): ModuleExports { const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); const importedNamesByLocalName = new Map(); + const importedSymbolsByLocalName = new Map(); + const namespaceImportsByLocalName = new Map(); const localConstDeclarations = new Map(); const localFunctions = new Map(); const directlyExportedNames = new Set(); const locallyExportedNames = new Set(); const exportedNames = new Set(); + const namedReExports: NamedReExport[] = []; + const pendingLocalReExports: Array<{ exportedName: string; localName: string }> = []; const starExportSpecifiers: string[] = []; for (const statement of sourceFile.statements) { @@ -251,16 +340,43 @@ export function collectModuleExportNames(content: string, fileName = "source.ts" if (bindings && ts.isNamedImports(bindings)) { for (const specifier of bindings.elements) { if (!statement.importClause?.isTypeOnly && !specifier.isTypeOnly) { - importedNamesByLocalName.set( - specifier.name.text, - specifier.propertyName?.text ?? specifier.name.text, - ); + const importedName = specifier.propertyName?.text ?? specifier.name.text; + const moduleSpecifier = ts.isStringLiteral(statement.moduleSpecifier) + ? statement.moduleSpecifier.text + : ""; + importedNamesByLocalName.set(specifier.name.text, importedName); + importedSymbolsByLocalName.set(specifier.name.text, { + importedName, + localName: specifier.name.text, + moduleSpecifier, + }); } } + } else if ( + bindings && + ts.isNamespaceImport(bindings) && + !statement.importClause?.isTypeOnly && + ts.isStringLiteral(statement.moduleSpecifier) + ) { + namespaceImportsByLocalName.set(bindings.name.text, statement.moduleSpecifier.text); } continue; } + if ( + ts.isImportEqualsDeclaration(statement) && + !statement.isTypeOnly && + ts.isExternalModuleReference(statement.moduleReference) && + statement.moduleReference.expression && + ts.isStringLiteral(statement.moduleReference.expression) + ) { + namespaceImportsByLocalName.set( + statement.name.text, + statement.moduleReference.expression.text, + ); + continue; + } + if (ts.isFunctionDeclaration(statement) && statement.name) { const name = statement.name.text; const declarations = localFunctions.get(name) ?? []; @@ -325,6 +441,15 @@ export function collectModuleExportNames(content: string, fileName = "source.ts" continue; } const localName = specifier.propertyName?.text ?? specifier.name.text; + if (statement.moduleSpecifier && ts.isStringLiteral(statement.moduleSpecifier)) { + namedReExports.push({ + exportedName: specifier.name.text, + importedName: localName, + moduleSpecifier: statement.moduleSpecifier.text, + }); + } else if (!statement.moduleSpecifier) { + pendingLocalReExports.push({ exportedName: specifier.name.text, localName }); + } // Renamed re-exports are deliberately outside this guard's first slice. if (specifier.name.text !== localName) { continue; @@ -337,11 +462,51 @@ export function collectModuleExportNames(content: string, fileName = "source.ts" } } + for (const reExport of pendingLocalReExports) { + const imported = importedSymbolsByLocalName.get(reExport.localName); + if (imported) { + namedReExports.push({ + exportedName: reExport.exportedName, + importedName: imported.importedName, + moduleSpecifier: imported.moduleSpecifier, + }); + } + } + const definitions = new Set(); + const valueDefinitions = new Map(); for (const name of new Set([...directlyExportedNames, ...locallyExportedNames])) { const constDeclarations = localConstDeclarations.get(name); if (constDeclarations) { const [constDeclaration] = constDeclarations; + if (constDeclarations.length === 1 && constDeclaration) { + const importedReferences = collectImportedReferences( + constDeclaration, + importedSymbolsByLocalName, + namespaceImportsByLocalName, + ); + const initializer = constDeclaration.initializer + ? unwrapExpression(constDeclaration.initializer) + : undefined; + if (initializer) { + const aliasSource = resolveImportedReference( + initializer, + importedSymbolsByLocalName, + namespaceImportsByLocalName, + ); + if (aliasSource) { + importedReferences.push(aliasSource); + } + } + valueDefinitions.set(name, { + importedReferences: importedReferences.toSorted((left, right) => + `${left.moduleSpecifier}\0${left.importedName}\0${left.localName}`.localeCompare( + `${right.moduleSpecifier}\0${right.importedName}\0${right.localName}`, + ), + ), + name, + }); + } if ( constDeclarations.length === 1 && constDeclaration && @@ -357,6 +522,16 @@ export function collectModuleExportNames(content: string, fileName = "source.ts" continue; } const implementation = functionDeclarations.find((declaration) => declaration.body); + if (implementation?.body) { + valueDefinitions.set(name, { + importedReferences: collectImportedReferences( + implementation.body, + importedSymbolsByLocalName, + namespaceImportsByLocalName, + ), + name, + }); + } // Lazy runtime facades are mandated by AGENTS.md. Exempt only exact same-name // argument forwarding so those boundaries do not become duplicate behavior. if ( @@ -368,10 +543,20 @@ export function collectModuleExportNames(content: string, fileName = "source.ts" definitions.add(name); } - return { definitions, exportedNames, starExportSpecifiers }; + return { + definitions, + exportedNames, + namedReExports: namedReExports.toSorted((left, right) => + `${left.exportedName}\0${left.importedName}\0${left.moduleSpecifier}`.localeCompare( + `${right.exportedName}\0${right.importedName}\0${right.moduleSpecifier}`, + ), + ), + starExportSpecifiers, + valueDefinitions, + }; } -function resolveStarExportPath( +export function resolveExportModulePath( sourcePath: string, specifier: string, modulesByPath: ReadonlyMap, @@ -411,7 +596,7 @@ function collectTransitiveExportNames( const nextVisiting = new Set(visiting).add(modulePath); const names = new Set(moduleExports.exportedNames); for (const specifier of moduleExports.starExportSpecifiers) { - const targetPath = resolveStarExportPath(modulePath, specifier, modulesByPath); + const targetPath = resolveExportModulePath(modulePath, specifier, modulesByPath); if (!targetPath) { continue; } diff --git a/scripts/check-wrapper-shadowing.mts b/scripts/check-wrapper-shadowing.mts new file mode 100644 index 000000000000..c252dc37a3ff --- /dev/null +++ b/scripts/check-wrapper-shadowing.mts @@ -0,0 +1,280 @@ +#!/usr/bin/env node + +import fs from "node:fs/promises"; +import path from "node:path"; +import { z } from "zod"; +import { + collectModuleExportNames, + isExcludedExportCollisionSource, + resolveExportModulePath, + type ModuleExports, + type SourceModule, +} from "./check-export-name-collisions.mts"; +import { resolveRepoRoot } from "./lib/repo-root.mjs"; +import { + collectTypeScriptFilesFromRoots, + resolveSourceRoots, + runAsScript, +} from "./lib/ts-guard-utils.mts"; + +export type WrapperShadowingViolation = { + name: string; + wrapped: string; + wrapper: string; + via?: string; +}; + +const violationSchema = z + .object({ + name: z.string(), + wrapped: z.string(), + wrapper: z.string(), + via: z.string().optional(), + }) + .strict(); +const baselineSchema = z.array(violationSchema); + +const baselineRelativePath = "scripts/lib/wrapper-shadowing-baseline.json"; +const baselineRegenCommand = "pnpm check:wrapper-shadowing:gen"; +const failurePrefix = "check-wrapper-shadowing"; + +function normalizeRelativePath(filePath: string) { + return filePath.replaceAll(path.sep, "/"); +} + +export function isExcludedWrapperShadowingSource(filePath: string) { + const normalized = normalizeRelativePath(filePath); + const segments = normalized.split("/"); + return ( + isExcludedExportCollisionSource(normalized) || + segments.some((segment) => + ["__mocks__", "__tests__", "test-helpers", "test-support"].includes(segment), + ) || + /-test-(?:helpers|support)\.[cm]?[jt]s$/u.test(normalized) + ); +} + +function compareViolations(left: WrapperShadowingViolation, right: WrapperShadowingViolation) { + return `${left.name}\0${left.wrapper}\0${left.wrapped}\0${left.via ?? ""}`.localeCompare( + `${right.name}\0${right.wrapper}\0${right.wrapped}\0${right.via ?? ""}`, + ); +} + +function violationKey(violation: WrapperShadowingViolation) { + return `${violation.name}\0${violation.wrapper}\0${violation.wrapped}\0${violation.via ?? ""}`; +} + +function resolveSourceModulePath( + sourcePath: string, + specifier: string, + modulesByPath: ReadonlyMap, +) { + const pluginSdkPrefix = specifier.startsWith("openclaw/plugin-sdk/") + ? "openclaw/plugin-sdk/" + : specifier.startsWith("@openclaw/plugin-sdk/") + ? "@openclaw/plugin-sdk/" + : null; + if (!pluginSdkPrefix) { + return resolveExportModulePath(sourcePath, specifier, modulesByPath); + } + return resolveExportModulePath( + "src/plugin-sdk/importer.ts", + `./${specifier.slice(pluginSdkPrefix.length)}`, + modulesByPath, + ); +} + +function resolveWrappedDefinition( + wrapperPath: string, + exportName: string, + moduleSpecifier: string, + modulesByPath: ReadonlyMap, +) { + const importedPath = resolveSourceModulePath(wrapperPath, moduleSpecifier, modulesByPath); + if (!importedPath) { + return null; + } + const importedModule = modulesByPath.get(importedPath); + if (!importedModule) { + return null; + } + if (importedModule.valueDefinitions.has(exportName)) { + return { wrapped: importedPath }; + } + + for (const reExport of importedModule.namedReExports) { + if (reExport.exportedName !== exportName || reExport.importedName !== exportName) { + continue; + } + const wrapped = resolveSourceModulePath(importedPath, reExport.moduleSpecifier, modulesByPath); + if (wrapped && modulesByPath.get(wrapped)?.valueDefinitions.has(exportName)) { + return { via: importedPath, wrapped }; + } + } + for (const reExportSpecifier of importedModule.starExportSpecifiers) { + const wrapped = resolveSourceModulePath(importedPath, reExportSpecifier, modulesByPath); + if (wrapped && modulesByPath.get(wrapped)?.valueDefinitions.has(exportName)) { + return { via: importedPath, wrapped }; + } + } + return null; +} + +/** Finds exported wrappers that shadow the same imported source symbol. */ +export function findWrapperShadowingViolations(modules: SourceModule[]) { + const modulesByPath = new Map(); + for (const sourceModule of modules.toSorted((left, right) => + left.path.localeCompare(right.path), + )) { + const modulePath = normalizeRelativePath(sourceModule.path); + modulesByPath.set(modulePath, collectModuleExportNames(sourceModule.content, modulePath)); + } + + const violations = new Map(); + for (const [wrapperPath, moduleExports] of modulesByPath) { + for (const [name, definition] of moduleExports.valueDefinitions) { + for (const reference of definition.importedReferences) { + if (reference.importedName !== name) { + continue; + } + const wrappedDefinition = resolveWrappedDefinition( + wrapperPath, + name, + reference.moduleSpecifier, + modulesByPath, + ); + if (!wrappedDefinition || wrappedDefinition.wrapped === wrapperPath) { + continue; + } + const violation: WrapperShadowingViolation = { + name, + wrapped: wrappedDefinition.wrapped, + wrapper: wrapperPath, + ...(wrappedDefinition.via ? { via: wrappedDefinition.via } : {}), + }; + violations.set(violationKey(violation), violation); + } + } + } + return [...violations.values()].toSorted(compareViolations); +} + +export async function collectRepositoryWrapperShadowing(repoRoot: string) { + const collectedFiles = await collectTypeScriptFilesFromRoots( + resolveSourceRoots(repoRoot, ["src"]), + { + fileExtensions: [".ts", ".mts", ".js", ".mjs"], + includeTests: true, + skipDirectories: ["test", "__fixtures__"], + }, + ); + const files = collectedFiles.filter((filePath) => !isExcludedWrapperShadowingSource(filePath)); + const modules = await Promise.all( + files.map(async (filePath) => ({ + content: await fs.readFile(filePath, "utf8"), + path: normalizeRelativePath(path.relative(repoRoot, filePath)), + })), + ); + return findWrapperShadowingViolations(modules); +} + +function resolveBaselinePath(repoRoot: string) { + return path.join(repoRoot, ...baselineRelativePath.split("/")); +} + +async function readBaseline(repoRoot: string) { + try { + return baselineSchema.parse( + JSON.parse(await fs.readFile(resolveBaselinePath(repoRoot), "utf8")), + ); + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "ENOENT") { + return null; + } + throw error; + } +} + +export function findNewWrapperShadowingViolations( + current: WrapperShadowingViolation[], + baseline: WrapperShadowingViolation[], +) { + const baselineKeys = new Set(baseline.map(violationKey)); + return current.filter((violation) => !baselineKeys.has(violationKey(violation))); +} + +export async function evaluateWrapperShadowing(repoRoot: string) { + const baseline = await readBaseline(repoRoot); + if (!baseline) { + return { + baseline: null, + current: await collectRepositoryWrapperShadowing(repoRoot), + regressions: [] as WrapperShadowingViolation[], + }; + } + const current = await collectRepositoryWrapperShadowing(repoRoot); + return { + baseline, + current, + regressions: findNewWrapperShadowingViolations(current, baseline), + }; +} + +async function writeBaseline(repoRoot: string) { + const violations = await collectRepositoryWrapperShadowing(repoRoot); + await fs.writeFile(resolveBaselinePath(repoRoot), `${JSON.stringify(violations, null, 2)}\n`); + return violations.length; +} + +export async function main( + repoRoot = resolveRepoRoot(import.meta.url), + argv = process.argv.slice(2), +) { + const updateBaseline = argv.includes("--update-debt-baseline"); + const unknownArgs = argv.filter((arg) => arg !== "--update-debt-baseline"); + if (unknownArgs.length > 0) { + console.error(`Unknown argument(s): ${unknownArgs.join(", ")}`); + return 2; + } + if (updateBaseline) { + const count = await writeBaseline(repoRoot); + console.log(`Wrote ${baselineRelativePath} (${count} entries)`); + return 0; + } + + const result = await evaluateWrapperShadowing(repoRoot); + if (!result.baseline) { + console.error( + `Missing ${baselineRelativePath}; run \`${baselineRegenCommand}\` and commit it.`, + ); + return 1; + } + if (result.regressions.length === 0) { + console.log( + `wrapper shadowing guard passed (${result.current.length} current, ${result.baseline.length} baselined).`, + ); + return 0; + } + + console.error(`Found new same-name wrapper shadowing beyond ${baselineRelativePath}:`); + for (const violation of result.regressions) { + console.error(`- ${JSON.stringify(violation)}`); + } + console.error( + "Keep the canonical name on the behavior-complete outer function; rename wrapped implementations with a distinguishing suffix, or use a pure re-export when no behavior is added.", + ); + return 1; +} + +runAsScript(import.meta.url, async () => { + let exitCode = 1; + try { + exitCode = await main(); + } catch (error) { + console.error(error); + } + if (exitCode !== 0) { + process.exitCode = exitCode; + console.error(`[${failurePrefix}] FAILED (exit ${exitCode})`); + } +}); diff --git a/scripts/check.mts b/scripts/check.mts index f026e621866c..c3a29f206389 100644 --- a/scripts/check.mts +++ b/scripts/check.mts @@ -71,6 +71,12 @@ export async function main(argv = process.argv.slice(2)) { args: ["check:deprecated-api-usage"], } : null, + !args.includeArchitecture + ? { + name: "wrapper shadowing guard", + args: ["check:wrapper-shadowing"], + } + : null, { name: "temp path guard", args: ["check:temp-path-guardrails"] }, { name: "pairing store guard", args: ["lint:auth:no-pairing-store-group"] }, { name: "pairing account guard", args: ["lint:auth:pairing-account-scope"] }, diff --git a/scripts/lib/wrapper-shadowing-baseline.json b/scripts/lib/wrapper-shadowing-baseline.json new file mode 100644 index 000000000000..306a57755b62 --- /dev/null +++ b/scripts/lib/wrapper-shadowing-baseline.json @@ -0,0 +1,393 @@ +[ + { + "name": "buildConfigSchema", + "wrapped": "src/config/schema.ts", + "wrapper": "src/config/doc-baseline.runtime.ts" + }, + { + "name": "buildShouldSuppressBuiltInModel", + "wrapped": "src/agents/model-suppression.ts", + "wrapper": "src/agents/model-suppression.runtime.ts" + }, + { + "name": "clearRuntimeAuthProfileStoreSnapshot", + "wrapped": "src/agents/auth-profiles/runtime-snapshots.ts", + "wrapper": "src/agents/auth-profiles/store.ts" + }, + { + "name": "clearRuntimeAuthProfileStoreSnapshots", + "wrapped": "src/agents/auth-profiles/runtime-snapshots.ts", + "wrapper": "src/agents/auth-profiles/store.ts" + }, + { + "name": "clearSecretsRuntimeSnapshot", + "wrapped": "src/secrets/runtime-state.ts", + "wrapper": "src/secrets/runtime.ts" + }, + { + "name": "collectBundledChannelConfigs", + "wrapped": "src/plugins/bundled-channel-config-metadata.ts", + "wrapper": "src/config/doc-baseline.runtime.ts" + }, + { + "name": "collectChannelSchemaMetadata", + "wrapped": "src/config/channel-config-metadata.ts", + "wrapper": "src/config/doc-baseline.runtime.ts" + }, + { + "name": "collectChannelSecurityFindings", + "wrapped": "src/security/audit-channel.ts", + "wrapper": "src/security/audit-channel.collect.runtime.ts" + }, + { + "name": "collectPluginSchemaMetadata", + "wrapped": "src/config/channel-config-metadata.ts", + "wrapper": "src/config/doc-baseline.runtime.ts" + }, + { + "name": "createAgentHarnessToolSurfaceRuntime", + "wrapped": "src/agents/harness/tool-surface-bridge.ts", + "wrapper": "src/plugin-sdk/agent-harness-tool-runtime.ts" + }, + { + "name": "createChannelPluginBase", + "wrapped": "src/plugin-sdk/core.ts", + "wrapper": "src/plugin-sdk/channel-core.ts" + }, + { + "name": "createDeferred", + "wrapped": "src/shared/deferred.ts", + "wrapper": "src/plugin-sdk/extension-shared.ts" + }, + { + "name": "createSessionSlug", + "wrapped": "src/agents/session-slug.ts", + "wrapper": "src/agents/bash-process-registry.ts" + }, + { + "name": "DEFAULT_PROGRESS_DRAFT_LABELS", + "wrapped": "src/shared/progress-labels.ts", + "wrapper": "src/channels/streaming.ts" + }, + { + "name": "drainPendingDeliveries", + "wrapped": "src/infra/outbound/delivery-queue-recovery.ts", + "wrapper": "src/plugin-sdk/delivery-queue-runtime.ts", + "via": "src/infra/outbound/delivery-queue.ts" + }, + { + "name": "formatSkillsForPrompt", + "wrapped": "src/skills/loading/skill-contract.ts", + "wrapper": "src/skills/loading/session.ts" + }, + { + "name": "getActivePluginRegistryWorkspaceDirFromState", + "wrapped": "src/plugins/runtime-workspace-state.ts", + "wrapper": "src/plugins/runtime-state.ts" + }, + { + "name": "getActiveRuntimeWebToolsMetadata", + "wrapped": "src/secrets/runtime-web-tools-state.ts", + "wrapper": "src/secrets/runtime.ts" + }, + { + "name": "getActiveSecretsRuntimeEnv", + "wrapped": "src/secrets/runtime-state.ts", + "wrapper": "src/secrets/runtime.ts" + }, + { + "name": "getActiveSecretsRuntimeSnapshotRevision", + "wrapped": "src/secrets/runtime-state.ts", + "wrapper": "src/secrets/runtime.ts" + }, + { + "name": "getActiveSecretsRuntimeSnapshot", + "wrapped": "src/secrets/runtime-state.ts", + "wrapper": "src/secrets/runtime.ts" + }, + { + "name": "getActiveSkillEnvKeys", + "wrapped": "src/skills/runtime/env-overrides.ts", + "wrapper": "src/skills/runtime/env-overrides.runtime.ts" + }, + { + "name": "getApiKeyForModel", + "wrapped": "src/agents/model-auth-model.ts", + "wrapper": "src/plugins/runtime/runtime-model-auth.runtime.ts", + "via": "src/agents/model-auth.ts" + }, + { + "name": "getCoreCliCommandNames", + "wrapped": "src/cli/program/core-command-descriptors.ts", + "wrapper": "src/cli/program/command-registry-core.ts" + }, + { + "name": "getDefaultLocalRoots", + "wrapped": "src/media/local-media-access.ts", + "wrapper": "src/plugins/runtime/runtime-web-channel-plugin.ts", + "via": "src/media/web-media.ts" + }, + { + "name": "getDiagnosticSessionStateCountForTest", + "wrapped": "src/logging/diagnostic-session-state.ts", + "wrapper": "src/logging/diagnostic.ts" + }, + { + "name": "getPreparedRuntimeAuthProfileStoreSnapshot", + "wrapped": "src/agents/auth-profiles/runtime-snapshots.ts", + "wrapper": "src/agents/auth-profiles/store.ts" + }, + { + "name": "getRuntimeAuthProfileStoreSnapshot", + "wrapped": "src/agents/auth-profiles/runtime-snapshots.ts", + "wrapper": "src/agents/auth-profiles/store.ts" + }, + { + "name": "getSubCliEntries", + "wrapped": "src/cli/program/subcli-descriptors.ts", + "wrapper": "src/cli/program/register.subclis-core.ts" + }, + { + "name": "hasExplicitPluginConfig", + "wrapped": "src/plugins/config-normalization-shared.ts", + "wrapper": "src/plugins/config-state.ts" + }, + { + "name": "hasUsableOAuthCredential", + "wrapped": "src/agents/auth-profiles/credential-state.ts", + "wrapper": "src/agents/auth-profiles/oauth-shared.ts" + }, + { + "name": "isAllowedParsedChatSender", + "wrapped": "src/channels/plugins/chat-target-prefixes.ts", + "wrapper": "src/plugin-sdk/allow-from.ts" + }, + { + "name": "isBundledChannelEnabledByChannelConfig", + "wrapped": "src/plugins/config-normalization-shared.ts", + "wrapper": "src/plugins/config-state.ts" + }, + { + "name": "isMissingPathError", + "wrapped": "src/infra/errno.ts", + "wrapper": "src/plugin-sdk/memory-host-event-export.ts", + "via": "src/infra/errors.ts" + }, + { + "name": "listAuthProfileStoreAgentDirs", + "wrapped": "src/secrets/auth-store-paths.ts", + "wrapper": "src/secrets/storage-scan.ts" + }, + { + "name": "loadBundledPluginPublicSurfaceModuleSync", + "wrapped": "src/plugin-sdk/facade-loader.ts", + "wrapper": "src/plugin-sdk/facade-runtime.ts" + }, + { + "name": "loadCombinedSessionStoreForGateway", + "wrapped": "src/config/sessions/combined-store-gateway.ts", + "wrapper": "src/plugin-sdk/session-transcript-hit.ts" + }, + { + "name": "loadGatewayTlsRuntime", + "wrapped": "src/infra/tls/gateway.ts", + "wrapper": "src/gateway/server/tls.ts" + }, + { + "name": "loadPluginManifestRegistry", + "wrapped": "src/plugins/manifest-registry.ts", + "wrapper": "src/config/doc-baseline.runtime.ts" + }, + { + "name": "loadWebMediaRaw", + "wrapped": "src/media/web-media.ts", + "wrapper": "src/plugins/runtime/runtime-web-channel-plugin.ts" + }, + { + "name": "loadWebMedia", + "wrapped": "src/media/web-media.ts", + "wrapper": "src/plugins/runtime/runtime-web-channel-plugin.ts" + }, + { + "name": "maybeApplyTtsToPayload", + "wrapped": "src/tts/tts-payload.ts", + "wrapper": "src/tts/tts.ts" + }, + { + "name": "normalizeContainerPath", + "wrapped": "src/agents/sandbox/path-utils.ts", + "wrapper": "src/agents/sandbox/remote-fs-bridge-paths.ts" + }, + { + "name": "normalizeCronRunDiagnostics", + "wrapped": "src/cron/run-diagnostics-normalize.ts", + "wrapper": "src/cron/run-diagnostics.ts" + }, + { + "name": "normalizeOutboundReplyPayload", + "wrapped": "src/infra/outbound/reply-payload-normalize.ts", + "wrapper": "src/plugin-sdk/reply-payload.ts" + }, + { + "name": "normalizePluginsConfigWithResolver", + "wrapped": "src/plugins/config-normalization-shared.ts", + "wrapper": "src/plugins/config-policy.ts" + }, + { + "name": "optimizeImageToJpeg", + "wrapped": "src/media/web-media.ts", + "wrapper": "src/plugins/runtime/runtime-web-channel-plugin.ts" + }, + { + "name": "persistSessionEntry", + "wrapped": "src/agents/command/attempt-execution.shared.ts", + "wrapper": "src/agents/command/session-helpers.ts" + }, + { + "name": "pruneStaleCommandPolls", + "wrapped": "src/agents/command-poll-backoff.ts", + "wrapper": "src/agents/command-poll-backoff.runtime.ts" + }, + { + "name": "readAmbientTranscriptWatermark", + "wrapped": "src/config/sessions/ambient-transcript-watermark.ts", + "wrapper": "src/plugin-sdk/session-store-runtime.ts" + }, + { + "name": "readBooleanParam", + "wrapped": "src/plugin-sdk/boolean-param.ts", + "wrapper": "src/infra/outbound/message-action-params.ts" + }, + { + "name": "readLatestSessionUsageFromTranscriptAsync", + "wrapped": "src/gateway/session-utils.fs.ts", + "wrapper": "src/gateway/session-transcript-readers.ts" + }, + { + "name": "readSessionTranscriptVisibleMessageDelta", + "wrapped": "src/config/sessions/session-accessor.sqlite-active-events.ts", + "wrapper": "src/plugin-sdk/session-transcript-runtime.ts", + "via": "src/config/sessions/session-accessor.ts" + }, + { + "name": "registerSubCliByName", + "wrapped": "src/cli/program/register.subclis-core.ts", + "wrapper": "src/cli/program/register.subclis.ts" + }, + { + "name": "registerSubCliCommands", + "wrapped": "src/cli/program/register.subclis-core.ts", + "wrapper": "src/cli/program/register.subclis.ts" + }, + { + "name": "replaceRuntimeAuthProfileStoreSnapshots", + "wrapped": "src/agents/auth-profiles/runtime-snapshots.ts", + "wrapper": "src/agents/auth-profiles/store.ts" + }, + { + "name": "resolveAllowedModelRef", + "wrapped": "src/agents/model-selection-resolve.ts", + "wrapper": "src/agents/model-selection.ts" + }, + { + "name": "resolveAllowlistModelKey", + "wrapped": "src/agents/model-selection-shared.ts", + "wrapper": "src/agents/model-selection.ts" + }, + { + "name": "resolveApiKeyForProvider", + "wrapped": "src/agents/model-auth-provider.ts", + "wrapper": "src/plugins/runtime/runtime-model-auth.runtime.ts", + "via": "src/agents/model-auth.ts" + }, + { + "name": "resolveEffectiveOAuthCredential", + "wrapped": "src/agents/auth-profiles/oauth-manager.ts", + "wrapper": "src/agents/auth-profiles/effective-oauth.ts" + }, + { + "name": "resolveHeartbeatPrompt", + "wrapped": "src/auto-reply/heartbeat.ts", + "wrapper": "src/infra/heartbeat-runner-config.ts" + }, + { + "name": "resolveMissingPluginCommandMessage", + "wrapped": "src/cli/run-main-policy.ts", + "wrapper": "src/cli/run-main.ts" + }, + { + "name": "resolvePluginProviders", + "wrapped": "src/plugins/providers.runtime.ts", + "wrapper": "src/plugins/provider-auth-choice.runtime.ts" + }, + { + "name": "resolvePluginSetupProvider", + "wrapped": "src/plugins/setup-registry.ts", + "wrapper": "src/plugins/provider-auth-choice.runtime.ts" + }, + { + "name": "resolveProviderPluginChoice", + "wrapped": "src/plugins/provider-wizard.ts", + "wrapper": "src/plugins/provider-auth-choice.runtime.ts" + }, + { + "name": "resolveSecretPlanTargetByPath", + "wrapped": "src/secrets/target-registry-query.ts", + "wrapper": "src/plugin-sdk/secret-ref-runtime.ts" + }, + { + "name": "resolveSessionFilePath", + "wrapped": "src/config/sessions/paths.ts", + "wrapper": "src/plugin-sdk/session-store-runtime.ts" + }, + { + "name": "resolveSessionKeyForRequest", + "wrapped": "src/agents/command/session.ts", + "wrapper": "src/commands/agent/session.runtime.ts" + }, + { + "name": "resolveSessionStoreEntry", + "wrapped": "src/config/sessions/store-entry.ts", + "wrapper": "src/plugin-sdk/session-store-runtime.ts" + }, + { + "name": "resolveSessionTranscriptActiveLeafEntryId", + "wrapped": "src/config/sessions/session-accessor.sqlite-message-cut.ts", + "wrapper": "src/config/sessions/session-accessor.message-cut.ts" + }, + { + "name": "resolveStorePath", + "wrapped": "src/config/sessions/paths.ts", + "wrapper": "src/plugin-sdk/session-store-runtime.ts" + }, + { + "name": "resolveThinkingDefaultForModel", + "wrapped": "src/auto-reply/thinking.shared.ts", + "wrapper": "src/auto-reply/thinking.ts" + }, + { + "name": "runProviderModelSelectedHook", + "wrapped": "src/plugins/provider-wizard.ts", + "wrapper": "src/plugins/provider-auth-choice.runtime.ts" + }, + { + "name": "runSecurityAudit", + "wrapped": "src/security/audit.ts", + "wrapper": "src/security/audit.runtime.ts" + }, + { + "name": "sha256HexPrefix", + "wrapped": "src/infra/crypto-digest.ts", + "wrapper": "src/logging/redact-identifier.ts" + }, + { + "name": "shouldSuppressBuiltInModel", + "wrapped": "src/agents/model-suppression.ts", + "wrapper": "src/agents/model-suppression.runtime.ts" + }, + { + "name": "textToSpeech", + "wrapped": "src/tts/tts-synthesis.ts", + "wrapper": "src/tts/tts.ts" + } +] diff --git a/test/scripts/changed-lanes.test.ts b/test/scripts/changed-lanes.test.ts index 4d0083d4a960..63c71596ba70 100644 --- a/test/scripts/changed-lanes.test.ts +++ b/test/scripts/changed-lanes.test.ts @@ -41,6 +41,7 @@ import { shouldRunPluginSdkSurfaceChecks, shouldRunSqliteSessionSchemaBaselineCheck, shouldRunTestTempCreationReport, + shouldRunWrapperShadowingCheck, createNpmLockGuardCommand, delegationFailedBeforeRunning, } from "../../scripts/check-changed.mts"; @@ -1469,6 +1470,7 @@ describe("scripts/changed-lanes", () => { "format changed files", "deprecated API usage", "plugin boundaries", + "wrapper shadowing", "package patch guard", // These live-Docker paths include `src/gateway/*.live.test.ts`, and the // full-tree knip scan sees test files, so a deleted last consumer can @@ -1651,6 +1653,7 @@ describe("scripts/changed-lanes", () => { "--import", "check:deprecated-api-usage", "plugins:boundary-report:ci", + "check:wrapper-shadowing", "deps:patches:check", "release-metadata:check", "android:version:check", @@ -1918,6 +1921,28 @@ describe("scripts/changed-lanes", () => { } }); + it("runs wrapper shadowing for source and guard-owner changes", () => { + expect( + shouldRunWrapperShadowingCheck([ + "src/channels/turn/run-channel-turn.ts", + "scripts/check-wrapper-shadowing.mts", + "scripts/check-export-name-collisions.mts", + "scripts/lib/wrapper-shadowing-baseline.json", + "scripts/lib/ts-guard-utils.mts", + "package.json", + ]), + ).toBe(true); + expect(shouldRunWrapperShadowingCheck(["docs/concepts/message-lifecycle.md"])).toBe(false); + + const plan = createChangedCheckPlan( + detectChangedLanes(["scripts/check-wrapper-shadowing.mts"]), + ); + expect(plan.commands).toContainEqual({ + name: "wrapper shadowing", + args: ["check:wrapper-shadowing"], + }); + }); + it("guards release metadata package changes to the top-level version field", () => { const dir = makeTempRepoRoot(tempDirs, "openclaw-release-metadata-"); git(dir, ["init", "-q", "--initial-branch=main"]); diff --git a/test/scripts/check-wrapper-shadowing.test.ts b/test/scripts/check-wrapper-shadowing.test.ts new file mode 100644 index 000000000000..af82146c25e3 --- /dev/null +++ b/test/scripts/check-wrapper-shadowing.test.ts @@ -0,0 +1,121 @@ +import { spawnSync } from "node:child_process"; +import fs from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { + evaluateWrapperShadowing, + type WrapperShadowingViolation, +} from "../../scripts/check-wrapper-shadowing.mts"; +import { withTempDir } from "../../src/test-utils/temp-dir.js"; + +const guardScriptPath = fileURLToPath( + new URL("../../scripts/check-wrapper-shadowing.mts", import.meta.url), +); + +type GuardFixture = { + baseline?: WrapperShadowingViolation[]; + files: Record; +}; + +async function runFixture(fixture: GuardFixture) { + return await withTempDir("openclaw-wrapper-shadowing-", async (repoRoot) => { + await Promise.all( + Object.entries(fixture.files).map(async ([repoPath, content]) => { + const filePath = path.join(repoRoot, repoPath); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, content); + }), + ); + const baselinePath = path.join(repoRoot, "scripts/lib/wrapper-shadowing-baseline.json"); + await fs.mkdir(path.dirname(baselinePath), { recursive: true }); + await fs.writeFile(baselinePath, `${JSON.stringify(fixture.baseline ?? [], null, 2)}\n`); + return await evaluateWrapperShadowing(repoRoot); + }); +} + +const directViolation: GuardFixture["files"] = { + "src/inner.ts": "export function runTask() { return 'inner'; }\n", + "src/outer.ts": [ + 'import { runTask as runTaskInner } from "./inner.js";', + "export function runTask() {", + " prepareTask();", + " return runTaskInner();", + "}", + ].join("\n"), +}; + +describe("wrapper shadowing guard", () => { + it("fails for a same-name wrapper around an imported implementation", async () => { + const result = await runFixture({ files: directViolation }); + + expect(result.regressions).toEqual([ + { name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }, + ]); + }); + + it("passes for a pure re-export", async () => { + const result = await runFixture({ + files: { + "src/inner.ts": "export function runTask() { return 'inner'; }\n", + "src/outer.ts": 'export { runTask } from "./inner.js";\n', + }, + }); + + expect(result.current).toEqual([]); + expect(result.regressions).toEqual([]); + }); + + it("passes for a baselined violation", async () => { + const violation = { name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }; + const result = await runFixture({ + baseline: [ + violation, + { name: "removedTask", wrapped: "src/old-inner.ts", wrapper: "src/old-outer.ts" }, + ], + files: directViolation, + }); + + expect(result.current).toEqual([violation]); + expect(result.regressions).toEqual([]); + }); + + it("fails for a new violation on top of the baseline", async () => { + const baseline = { name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" }; + const result = await runFixture({ + baseline: [baseline], + files: { + ...directViolation, + "src/barrel.ts": 'export { sendTask } from "./sender.js";\n', + "src/sender.ts": "export const sendTask = () => 'sent';\n", + "src/send-wrapper.ts": [ + 'import { sendTask as sendTaskInner } from "./barrel.js";', + "export const sendTask = (...args: unknown[]) => {", + " recordSend();", + " return sendTaskInner(...args);", + "};", + ].join("\n"), + }, + }); + + expect(result.regressions).toEqual([ + { + name: "sendTask", + wrapped: "src/sender.ts", + wrapper: "src/send-wrapper.ts", + via: "src/barrel.ts", + }, + ]); + }); + + it("ends failures with the wrapper trailer", () => { + const result = spawnSync(process.execPath, ["--import", "tsx", guardScriptPath, "--invalid"], { + encoding: "utf8", + }); + + expect(result.status).toBe(2); + expect(result.stderr.trimEnd().split("\n").at(-1)).toBe( + "[check-wrapper-shadowing] FAILED (exit 2)", + ); + }); +});