mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
refactor: consolidate coercion contracts (#122458)
* refactor: consolidate coercion contracts Centralize exact string, record, numeric, date, Boolean, argument, and structured-error coercions while preserving call-site semantics. Migrate canonical-name collisions and deprecated internal SDK bypasses, deleting 55 net production/tooling lines. Expand declaration ownership enforcement to 101 allowed helpers and add a narrow export-completeness audit. * fix: preserve standalone script coercions Keep copied Control UI tooling self-contained and retain the trusted release harness module-relative source seam when the harness runs against an old target cwd.
This commit is contained in:
Peter Steinberger
committed by
GitHub
GitHub
parent
66fe424590
commit
b080dd1e76
@@ -2,13 +2,84 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
booleanFlag,
|
||||
classifyBoundedUnsignedDecimal,
|
||||
intFlag,
|
||||
parseFlagArgs,
|
||||
parsePermissiveBooleanToken,
|
||||
parseStrictBooleanArg,
|
||||
readFlagValue,
|
||||
stringFlag,
|
||||
stringListFlag,
|
||||
} from "../../scripts/lib/arg-utils.runtime.mjs";
|
||||
|
||||
describe("scripts/lib/arg-utils strict scalar grammars", () => {
|
||||
it.each([
|
||||
{ input: "true", expected: true },
|
||||
{ input: "false", expected: false },
|
||||
{ input: "", error: "--enabled must be true or false." },
|
||||
{ input: " ", error: "--enabled must be true or false." },
|
||||
{ input: " true", error: "--enabled must be true or false." },
|
||||
{ input: "false ", error: "--enabled must be true or false." },
|
||||
{ input: "TRUE", error: "--enabled must be true or false." },
|
||||
{ input: "False", error: "--enabled must be true or false." },
|
||||
{ input: "1", error: "--enabled must be true or false." },
|
||||
{ input: "0", error: "--enabled must be true or false." },
|
||||
{ input: "yes", error: "--enabled must be true or false." },
|
||||
{ input: true, error: "--enabled must be true or false." },
|
||||
{ input: 1, error: "--enabled must be true or false." },
|
||||
])("parses strict Boolean token %#", ({ input, expected, error }) => {
|
||||
if (error) {
|
||||
expect(() => parseStrictBooleanArg(input, "--enabled")).toThrow(error);
|
||||
return;
|
||||
}
|
||||
expect(parseStrictBooleanArg(input, "--enabled")).toBe(expected);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ input: "0", min: 0, max: 10, expected: { kind: "value", value: 0 } },
|
||||
{ input: "001", min: 1, max: 10, expected: { kind: "value", value: 1 } },
|
||||
{ input: "10", min: 0, max: 10, expected: { kind: "value", value: 10 } },
|
||||
{ input: "0", min: 1, max: 10, expected: { kind: "below" } },
|
||||
{ input: "11", min: 0, max: 10, expected: { kind: "above" } },
|
||||
{ input: "9".repeat(400), min: 0, max: 10, expected: { kind: "above" } },
|
||||
{ input: "", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: " ", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: " 1", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "1 ", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "+1", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "-1", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "1.0", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "1e1", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "0x10", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "0b10", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
{ input: "1ms", min: 0, max: 10, expected: { kind: "syntax" } },
|
||||
])("classifies bounded unsigned decimal %#", ({ input, min, max, expected }) => {
|
||||
expect(classifyBoundedUnsignedDecimal(input, min, max)).toEqual(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe("scripts/lib/arg-utils permissive Boolean tokens", () => {
|
||||
it.each([
|
||||
{ input: "true", expected: true },
|
||||
{ input: "1", expected: true },
|
||||
{ input: "yes", expected: true },
|
||||
{ input: "on", expected: true },
|
||||
{ input: "false", expected: false },
|
||||
{ input: "0", expected: false },
|
||||
{ input: "no", expected: false },
|
||||
{ input: "off", expected: false },
|
||||
{ input: " TRUE ", expected: true },
|
||||
{ input: " Off ", expected: false },
|
||||
{ input: "", expected: undefined },
|
||||
{ input: " ", expected: undefined },
|
||||
{ input: "enabled", expected: undefined },
|
||||
{ input: true, expected: undefined },
|
||||
{ input: 1, expected: undefined },
|
||||
])("parses $input as $expected", ({ input, expected }) => {
|
||||
expect(parsePermissiveBooleanToken(input)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe("scripts/lib/arg-utils parseFlagArgs", () => {
|
||||
it("uses the last value when a flag is repeated", () => {
|
||||
expect(readFlagValue(["-p", "first.json", "-p", "second.json"], "-p")).toBe("second.json");
|
||||
|
||||
@@ -3,8 +3,10 @@ import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
auditCanonicalCoercionExports,
|
||||
auditCoercionHelperDeclarations,
|
||||
findBannedCoercionHelperDeclarations,
|
||||
findExportedCallableNames,
|
||||
isGovernedCoercionHelperPath,
|
||||
runCoercionHelperDeclarationGuard,
|
||||
type CoercionHelperCarveOut,
|
||||
@@ -240,6 +242,68 @@ describe("coercion helper declaration AST guard", () => {
|
||||
expect(isGovernedCoercionHelperPath(".github/actions/example/index.ts")).toBe(true);
|
||||
});
|
||||
|
||||
it("finds directly exported callable declarations and export aliases", () => {
|
||||
const source = [
|
||||
"export function canonical() {}",
|
||||
"function local() {}",
|
||||
"export { local as alias };",
|
||||
"export const VALUE = 1;",
|
||||
].join("\n");
|
||||
|
||||
expect(findExportedCallableNames(source, "src/owner.ts")).toEqual(["alias", "canonical"]);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["classified", ["kept"], [{ file: "src/owner.ts", name: "kept", status: "enforced" }]],
|
||||
[
|
||||
"deferred",
|
||||
["format"],
|
||||
[
|
||||
{
|
||||
file: "src/owner.ts",
|
||||
name: "format",
|
||||
status: "deferred",
|
||||
reason: "Meaningful public collision.",
|
||||
},
|
||||
],
|
||||
],
|
||||
] as const)("accepts a %s canonical export", (_label, names, classifications) => {
|
||||
expect(
|
||||
auditCanonicalCoercionExports(new Map([["src/owner.ts", names]]), classifications),
|
||||
).toEqual({
|
||||
invalidClassifications: [],
|
||||
staleClassifications: [],
|
||||
unclassifiedExports: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("reports unclassified exports and stale, duplicate, or blank deferred entries", () => {
|
||||
const kept = { file: "src/owner.ts", name: "kept", status: "enforced" } as const;
|
||||
const removed = {
|
||||
file: "src/owner.ts",
|
||||
name: "removed",
|
||||
status: "deferred",
|
||||
reason: "Removed owner.",
|
||||
} as const;
|
||||
const blank = {
|
||||
file: "src/other.ts",
|
||||
name: "unknown",
|
||||
status: "deferred",
|
||||
reason: "",
|
||||
} as const;
|
||||
const audit = auditCanonicalCoercionExports(
|
||||
new Map([["src/owner.ts", ["kept", "newHelper"]]]),
|
||||
[kept, kept, removed, blank],
|
||||
);
|
||||
|
||||
expect(audit.invalidClassifications).toEqual([
|
||||
"src/owner.ts [kept] is classified more than once",
|
||||
"src/other.ts [unknown] needs a non-empty deferred reason",
|
||||
]);
|
||||
expect(audit.unclassifiedExports).toEqual([{ file: "src/owner.ts", name: "newHelper" }]);
|
||||
expect(audit.staleClassifications).toEqual([removed, blank]);
|
||||
});
|
||||
|
||||
it("scans a temporary repository and reports sorted, owner-specific diagnostics", () => {
|
||||
const repoRoot = tempDirs.make("coercion-helper-guard-");
|
||||
fs.mkdirSync(path.join(repoRoot, "src"), { recursive: true });
|
||||
|
||||
@@ -65,7 +65,7 @@ describe("scripts/check-deprecated-api-usage", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("bans internal imports of every deprecated reply facade", () => {
|
||||
it("bans internal imports of every deprecated facade", () => {
|
||||
const modulePaths = new Set(
|
||||
BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES.map((ban) => ban.modulePath),
|
||||
);
|
||||
@@ -74,6 +74,7 @@ describe("scripts/check-deprecated-api-usage", () => {
|
||||
"src/plugin-sdk/channel-message",
|
||||
"src/plugin-sdk/channel-reply-pipeline",
|
||||
"src/plugin-sdk/inbound-reply-dispatch",
|
||||
"src/plugin-sdk/text-runtime",
|
||||
]) {
|
||||
expect(modulePaths.has(facade), facade).toBe(true);
|
||||
}
|
||||
@@ -95,6 +96,7 @@ describe("scripts/check-deprecated-api-usage", () => {
|
||||
'import { createChannelReplyPipeline } from "openclaw/plugin-sdk/channel-reply-pipeline";',
|
||||
'export { runChannelInboundEvent } from "../plugin-sdk/inbound-reply-dispatch.js";',
|
||||
'const facade = await import ("../plugin-sdk/channel-message.js", { with: {} });',
|
||||
'const text = require("@openclaw/plugin-sdk/text-runtime");',
|
||||
].join("\n"),
|
||||
});
|
||||
|
||||
@@ -106,6 +108,7 @@ describe("scripts/check-deprecated-api-usage", () => {
|
||||
"src/channels/probe.ts:2: ../plugin-sdk/inbound-reply-dispatch.js",
|
||||
);
|
||||
expect(result.stderr).toContain("src/channels/probe.ts:3: ../plugin-sdk/channel-message.js");
|
||||
expect(result.stderr).toContain("src/channels/probe.ts:4: @openclaw/plugin-sdk/text-runtime");
|
||||
});
|
||||
|
||||
it("allows canonical compat re-exports and test files", () => {
|
||||
|
||||
Reference in New Issue
Block a user