refactor(tooling): enforce zero wrapper and export debt (#123020)

This commit is contained in:
Peter Steinberger
2026-08-13 00:40:32 -07:00
committed by GitHub
parent 732b3da091
commit fce5b6688a
9 changed files with 62 additions and 306 deletions
-2
View File
@@ -1522,7 +1522,6 @@
"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",
@@ -1649,7 +1648,6 @@
"lint:tmp:channel-agnostic-boundaries": "node --import tsx scripts/check-channel-agnostic-boundaries.mts",
"lint:tmp:dynamic-import-warts": "node --import tsx scripts/check-dynamic-import-warts.mts",
"lint:tmp:export-name-collisions": "node --import tsx scripts/check-export-name-collisions.mts",
"lint:tmp:export-name-collisions:gen": "node --import tsx scripts/check-export-name-collisions.mts --update-debt-baseline",
"lint:tmp:no-random-messaging": "node --import tsx scripts/check-no-random-messaging-tmp.mts",
"lint:tmp:no-raw-channel-fetch": "node --import tsx scripts/check-no-raw-channel-fetch.mts",
"lint:tmp:no-raw-http2-imports": "node --import tsx scripts/check-no-raw-http2-imports.mts",
+1 -1
View File
@@ -107,7 +107,7 @@ const PLUGIN_SDK_SURFACE_PATH_RE =
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;
/^(?:package\.json$|src\/|scripts\/(?:check-(?:export-name-collisions|wrapper-shadowing)\.mts$|lib\/ts-guard-utils\.mts$))/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 =
+12 -115
View File
@@ -3,7 +3,6 @@
import fs from "node:fs/promises";
import path from "node:path";
import ts from "typescript";
import { z } from "zod";
import { resolveRepoRoot } from "./lib/repo-root.mjs";
import {
collectTypeScriptFilesFromRoots,
@@ -57,17 +56,6 @@ export type ModuleExports = {
valueDefinitions: Map<string, ExportedValueDefinition>;
};
const exportNameCollisionSchema = z
.object({
name: z.string(),
files: z.array(z.string()),
sdk: z.literal(true).optional(),
})
.strict();
const exportNameCollisionBaselineSchema = z.array(exportNameCollisionSchema);
const baselineRelativePath = "scripts/lib/export-name-collision-baseline.json";
const baselineRegenCommand = "pnpm lint:tmp:export-name-collisions:gen";
const failurePrefix = "check-export-name-collisions";
const extraExcludedFileSuffixes = [".test-support.ts", ".test-helpers.ts", ".d.ts"];
@@ -695,51 +683,6 @@ export function findExportNameCollisions(modules: SourceModule[]): ExportNameCol
return analyzeExportNames(modules).collisions;
}
type CollisionChange = {
baseline?: ExportNameCollision;
current?: ExportNameCollision;
};
/** Compares every collision cluster so additions fail and removals ratchet debt down. */
export function compareExportNameCollisionDebt(
current: ExportNameCollision[],
baseline: ExportNameCollision[],
) {
const currentByName = new Map(current.map((collision) => [collision.name, collision]));
const baselineByName = new Map(baseline.map((collision) => [collision.name, collision]));
const regressions: CollisionChange[] = [];
const improvements: CollisionChange[] = [];
const names = [...new Set([...currentByName.keys(), ...baselineByName.keys()])].toSorted();
for (const name of names) {
const currentCollision = currentByName.get(name);
const baselineCollision = baselineByName.get(name);
if (!baselineCollision) {
regressions.push({ current: currentCollision });
continue;
}
if (!currentCollision) {
improvements.push({ baseline: baselineCollision });
continue;
}
const baselineFiles = new Set(baselineCollision.files);
const currentFiles = new Set(currentCollision.files);
const hasAddedFile = currentCollision.files.some((file) => !baselineFiles.has(file));
const hasRemovedFile = baselineCollision.files.some((file) => !currentFiles.has(file));
if (hasAddedFile || (currentCollision.sdk === true && baselineCollision.sdk !== true)) {
regressions.push({ baseline: baselineCollision, current: currentCollision });
}
if (hasRemovedFile || (baselineCollision.sdk === true && currentCollision.sdk !== true)) {
improvements.push({ baseline: baselineCollision, current: currentCollision });
}
}
return { regressions, improvements };
}
function resolveBaselinePath(repoRoot: string) {
return path.join(repoRoot, ...baselineRelativePath.split("/"));
}
async function collectRepositoryModules(repoRoot: string) {
const sourceCollectOptions = {
fileExtensions: [".ts", ".mts", ".js", ".mjs"],
@@ -784,28 +727,6 @@ export async function collectRepositoryCollisions(repoRoot: string) {
return (await collectRepositoryExportAnalysis(repoRoot)).collisions;
}
async function readBaseline(repoRoot: string) {
try {
return exportNameCollisionBaselineSchema.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;
}
}
async function writeBaseline(repoRoot: string, collisions: ExportNameCollision[]) {
await fs.writeFile(resolveBaselinePath(repoRoot), `${JSON.stringify(collisions, null, 2)}\n`);
return collisions.length;
}
function formatCollision(collision: ExportNameCollision | undefined) {
return JSON.stringify(collision);
}
function printAliasingReExports(reExports: AliasingReExport[]) {
if (reExports.length === 0) {
return;
@@ -818,51 +739,27 @@ function printAliasingReExports(reExports: AliasingReExport[]) {
}
}
export async function main() {
const repoRoot = resolveRepoRoot(import.meta.url);
if (process.argv.includes("--update-debt-baseline")) {
const analysis = await collectRepositoryExportAnalysis(repoRoot);
const count = await writeBaseline(repoRoot, analysis.collisions);
console.log(`Wrote ${baselineRelativePath} (${count} entries)`);
printAliasingReExports(analysis.aliasingReExports);
return 0;
export async function main(
repoRoot = resolveRepoRoot(import.meta.url),
argv = process.argv.slice(2),
) {
if (argv.length > 0) {
console.error(`Unknown argument(s): ${argv.join(", ")}`);
return 2;
}
const baseline = await readBaseline(repoRoot);
if (!baseline) {
console.error(
`Missing ${baselineRelativePath}; run \`${baselineRegenCommand}\` and commit it.`,
);
return 1;
}
const analysis = await collectRepositoryExportAnalysis(repoRoot);
const debt = compareExportNameCollisionDebt(analysis.collisions, baseline);
printAliasingReExports(analysis.aliasingReExports);
if (debt.regressions.length === 0 && debt.improvements.length === 0) {
if (analysis.collisions.length === 0) {
console.log("export name collision guard passed.");
return 0;
}
if (debt.regressions.length > 0) {
console.error(
`Found new exported function/const name collisions beyond ${baselineRelativePath}:`,
);
for (const regression of debt.regressions) {
console.error(`- ${formatCollision(regression.current)}`);
}
console.error(
`Give each behavior one exported spelling. If the debt increase is intentional, run \`${baselineRegenCommand}\` and commit the generated baseline.`,
);
}
if (debt.improvements.length > 0) {
console.error(`Export name collision debt dropped below ${baselineRelativePath}:`);
for (const improvement of debt.improvements) {
console.error(
`- ${improvement.baseline?.name}: ${formatCollision(improvement.baseline)} -> ${formatCollision(improvement.current)}`,
);
}
console.error(`Run \`${baselineRegenCommand}\` to ratchet the baseline down and commit it.`);
console.error("Found exported function/const name collisions:");
for (const collision of analysis.collisions) {
console.error(`- ${JSON.stringify(collision)}`);
}
console.error("Give each behavior one exported spelling.");
return 1;
}
+8 -84
View File
@@ -2,7 +2,6 @@
import fs from "node:fs/promises";
import path from "node:path";
import { z } from "zod";
import {
collectModuleExportNames,
isExcludedExportCollisionSource,
@@ -24,18 +23,6 @@ export type WrapperShadowingViolation = {
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) {
@@ -178,86 +165,23 @@ export async function collectRepositoryWrapperShadowing(repoRoot: string) {
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(", ")}`);
if (argv.length > 0) {
console.error(`Unknown argument(s): ${argv.join(", ")}`);
return 2;
}
if (updateBaseline) {
const count = await writeBaseline(repoRoot);
console.log(`Wrote ${baselineRelativePath} (${count} entries)`);
const violations = await collectRepositoryWrapperShadowing(repoRoot);
if (violations.length === 0) {
console.log("wrapper shadowing guard passed.");
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("Found same-name wrapper shadowing:");
for (const violation of violations) {
console.error(`- ${JSON.stringify(violation)}`);
}
console.error(
@@ -1 +0,0 @@
[]
@@ -1 +0,0 @@
[]
+7 -2
View File
@@ -1881,12 +1881,17 @@ describe("scripts/changed-lanes", () => {
"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);
expect(
shouldRunWrapperShadowingCheck([
"docs/concepts/message-lifecycle.md",
"scripts/lib/wrapper-shadowing-baseline.json",
"scripts/lib/export-name-collision-baseline.json",
]),
).toBe(false);
const plan = createChangedCheckPlan(
detectChangedLanes(["scripts/check-wrapper-shadowing.mts"]),
@@ -1,16 +1,21 @@
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 {
collectModuleExportNames,
collectRepositoryCollisions,
compareExportNameCollisionDebt,
findAliasingReExports,
findExportNameCollisions,
isExcludedExportCollisionSource,
} from "../../scripts/check-export-name-collisions.mts";
import { withTempDir } from "../../src/test-utils/temp-dir.js";
const guardScriptPath = fileURLToPath(
new URL("../../scripts/check-export-name-collisions.mts", import.meta.url),
);
describe("export name collision guard", () => {
it.each([
["src/example.test.ts", true],
@@ -244,34 +249,17 @@ describe("export name collision guard", () => {
},
]);
});
});
describe("export name collision debt baseline", () => {
it("separates new debt from baseline improvements", () => {
expect(
compareExportNameCollisionDebt(
[
{ name: "added", files: ["src/a.ts", "src/b.ts"] },
{ name: "expanded", files: ["src/a.ts", "src/b.ts", "src/c.ts"], sdk: true },
],
[
{ name: "expanded", files: ["src/a.ts", "src/b.ts"] },
{ name: "removed", files: ["src/c.ts", "src/d.ts"] },
],
),
).toEqual({
regressions: [
{ current: { name: "added", files: ["src/a.ts", "src/b.ts"] } },
{
baseline: { name: "expanded", files: ["src/a.ts", "src/b.ts"] },
current: {
name: "expanded",
files: ["src/a.ts", "src/b.ts", "src/c.ts"],
sdk: true,
},
},
],
improvements: [{ baseline: { name: "removed", files: ["src/c.ts", "src/d.ts"] } }],
});
it("rejects debt-baseline updates with the collision trailer", () => {
const result = spawnSync(
process.execPath,
["--import", "tsx", guardScriptPath, "--update-debt-baseline"],
{ encoding: "utf8" },
);
expect(result.status).toBe(2);
expect(result.stderr.trimEnd().split("\n").at(-1)).toBe(
"[check-export-name-collisions] FAILED (exit 2)",
);
});
});
+17 -71
View File
@@ -3,38 +3,29 @@ 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 { collectRepositoryWrapperShadowing } 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<string, string>;
};
type GuardFixture = Record<string, string>;
async function runFixture(fixture: GuardFixture) {
async function runFixture(files: GuardFixture) {
return await withTempDir("openclaw-wrapper-shadowing-", async (repoRoot) => {
await Promise.all(
Object.entries(fixture.files).map(async ([repoPath, content]) => {
Object.entries(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);
return await collectRepositoryWrapperShadowing(repoRoot);
});
}
const directViolation: GuardFixture["files"] = {
const directViolation: GuardFixture = {
"src/inner.ts": "export function runTask() { return 'inner'; }\n",
"src/outer.ts": [
'import { runTask as runTaskInner } from "./inner.js";',
@@ -47,71 +38,26 @@ const directViolation: GuardFixture["files"] = {
describe("wrapper shadowing guard", () => {
it("fails for a same-name wrapper around an imported implementation", async () => {
const result = await runFixture({ files: directViolation });
const result = await runFixture(directViolation);
expect(result.regressions).toEqual([
{ name: "runTask", wrapped: "src/inner.ts", wrapper: "src/outer.ts" },
]);
expect(result).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',
},
"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([]);
expect(result).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",
});
it("rejects debt-baseline updates with the wrapper trailer", () => {
const result = spawnSync(
process.execPath,
["--import", "tsx", guardScriptPath, "--update-debt-baseline"],
{ encoding: "utf8" },
);
expect(result.status).toBe(2);
expect(result.stderr.trimEnd().split("\n").at(-1)).toBe(